Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Doc] Moving file while using --new-from-rev #4349

Open
5 tasks done
amenasria opened this issue Feb 6, 2024 · 2 comments
Open
5 tasks done

[Doc] Moving file while using --new-from-rev #4349

amenasria opened this issue Feb 6, 2024 · 2 comments

Comments

@amenasria
Copy link

amenasria commented Feb 6, 2024

Welcome

Description of the problem

We recently used the new-from-rev key in our configuration to enable revive on our large codebase to avoid having to fix all the errors directly. It was really practical at first !

One major caveat of new-from-rev imo is moving files. Let's say I have a legacy file hello.go with 100 lints (i.e linter errors). The new-from-rev parameter will silence them all. But if I rename this file to hello_world.go or move it to another folder then all the lints will rise again.

I think this should be mentioned in https://golangci-lint.run/usage/faq/#how-to-integrate-golangci-lint-into-large-project-with-thousands-of-issues to make people aware of the cost of relying on new-from-rev.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version v1.54.2 built with go1.21.5 from (unknown, mod sum: "h1:oR9zxfWYxt7hFqk6+fw6Enr+E7F0SN2nqHhJYyIb0yo=") on (unknown)

Configuration

https://github.com/DataDog/datadog-agent/blob/fcb19ce078e7969d285565beec5d374c5fd623e1/.golangci.yml

Go environment

$ go version && go env
go version go1.21.5 darwin/arm64

Validation

  • Yes, I've included all information above (version, config, etc.).
@amenasria amenasria added the bug Something isn't working label Feb 6, 2024
@amenasria amenasria changed the title Moving file while using --new-from-rev [Doc] Moving file while using --new-from-rev Feb 6, 2024
@ldez ldez added enhancement New feature or improvement area: docs and removed bug Something isn't working labels Feb 7, 2024
@ldez ldez added question Further information is requested area: docs and removed enhancement New feature or improvement area: docs question Further information is requested labels Apr 24, 2024
@ldez
Copy link
Member

ldez commented Apr 24, 2024

Hello,

It feel expected: new-from-rev will use a git diff, so when you move a file, this file is inside the diff.

I don't know how to improve the documentation about that 🤔

@amenasria
Copy link
Author

amenasria commented Jun 18, 2024

Hey, sorry for the late response I missed the notification !

It feel expected: new-from-rev will use a git diff, so when you move a file, this file is inside the diff.

I agree that this is expected because of the use of git diff, the thing is it's not mentioned anywhere that it's using a git diff, I understood it while playing with it. This is imo what hides the moving-file-trap.

I don't know how to improve the documentation about that 🤔

I think I would expect something like:

Warning: the `--new-from-rev: REV` key works by running a `git diff` between the git revision REV and the current commit.

Renaming / moving a file will hence reveal all the silenced lints in this file.

WDYT? Does this seem reasonable to you?

Thanks for taking the time to answer !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants