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

discuss about design deprecation cycle (how to handle deprecated linter) #1987

Closed
sanposhiho opened this issue May 16, 2021 · 7 comments · Fixed by #4474
Closed

discuss about design deprecation cycle (how to handle deprecated linter) #1987

sanposhiho opened this issue May 16, 2021 · 7 comments · Fixed by #4474
Labels
enhancement New feature or improvement

Comments

@sanposhiho
Copy link
Member

sanposhiho commented May 16, 2021

Is your feature request related to a problem? Please describe.

Now, Some linters have been deprecated on golangci-lint.
Golangci-lint send out a warn message to users who are using deprecated linter, telling them that it is deprecated.

The number of deprecated linters is expected to increase in the future.
We need to discuss how to handle deprecated linter.

Describe the solution you'd like

  • It will be deprecated for a while and then removed from golangci-lint?
  • For users who want to use a deprecated linter for some reason, leave it deprecated and leave it in golangci instead of deleting it?
  • Create new status like deleted, separate from deprecated?
  • (Other opinions are welcome)

Additional context

had a small discussion on #1986 about how to handle deprecated linter when the enable-all is specified

  • If enable-all automatically avoids deprecated linter, existing users will not be able to notice that the linter they were using through enable-all has been deprecated.
  • But, from the point of view of new golangci-lint users, in order to avoid using deprecated linters, they need to use enable-all with many disable options.
    • This concern may not be necessary if deprecated linters will be removed from Golangci-lint finally. (This is because users only need to disable the linter that has been recently deprecated and has not yet been removed from golangci.)
@sanposhiho sanposhiho added the enhancement New feature or improvement label May 16, 2021
@sanposhiho
Copy link
Member Author

sanposhiho commented May 16, 2021

Create new status like deleted, separate from deprecated

I support this idea I mentioned above.

Users who want to use a linter that has been removed for some reason can use the linter.

As for the specification of the deleted status linter, I am thinking of the following

  • linters with status deleted are not enabled with enable-all
    • User will be able to notice that the linter has been removed by the WARN message that appeared when its state is deprecated.
  • In order to make users aware that they are using a deleted linter, it might be a good idea to enable it only with options like enable-deleted.
    • We can send a WARN message to a user who is using a linter with status "deleted" with enable option, saying "This linter has been deleted, if you still want to enable it, please use the enable-deleted option". This will make the user stop using it unless they have a strong will to do so.

@ldez
Copy link
Member

ldez commented May 16, 2021

A standard deprecation cycle is: available -> deprecated -> removed

The questions behind this cycle are:

  • how long is the deprecation phase? (ex: 1 month, 2 minor versions, until the next major version, ...)
  • when the drop of the element is done? (ex: in a minor version, in a major version, ...)

During the deprecation phase, it possible to have several steps:

  1. display a warning message: you can still use the item but you have a message
  2. display an error message: there is an error to force the user to remove the item

The last phase (remove) is breaking, it's expected and it must clear: when something is removed it must be removed.

About phase durations, for me, it's better to link this to a version (minor or major) instead of months or a time duration.

Now, the main question: do we want to drop an item in a minor version or in a major version?

@sanposhiho
Copy link
Member Author

it's expected and it must clear

Right. So, do you mean that we should erase it completely from the golangci-lint code instead of leaving it on code as deleted state?

it's better to link this to a version (minor or major) instead of months or a time duration.

agree :D

do we want to drop an item in a minor version or in a major version?

I think we should remove deprecated linters in the major version according to Semantic Versioning 2.0.0.

The major version must be incremented and the minor and patch versions must be set to zero after a backwards incompatible change is made to the module's public interface or documented functionality, for example, after a package is removed.
https://golang.org/ref/mod#versions

@knweiss
Copy link

knweiss commented Jan 7, 2022

May I also suggest that deprecated linters should be removed from the various --presets sets?

Example:

$ golangci-lint run -p bugs
WARN [runner] The linter 'scopelint' is deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner.  Replaced by exportloopref. 

As I did not configure an explicit list of linters (but rely on the presets instead) the set of enabled linters should IMHO be free of deprecated linters that cause such warnings.

Searching for Deprecated in golangci-lint/pkg/lint/lintersdb/manager.go shows several of these cases.

@yangyile1990

This comment was marked as off-topic.

@yangyile1990

This comment was marked as off-topic.

@ldez
Copy link
Member

ldez commented Mar 3, 2024

My proposal is still nearly the same:

  1. display a warning message: you can still use the item, but you have a message
  2. display an error message: there is an error to force the user to remove the item (also the implementation is replaced by the Noop linter)
  3. remove them

Each step is a minor version, ex:

  1. v1.100.0 -> warning
  2. v1.101.0 -> error
  3. v1.102.0 -> remove

Currently, we consider linters changes (like options that are breaking) like non-breaking changes for golangci-lint itself.
But we have to provide clear information about those changes (changelog, logs, public communication, etc.).
I think the lifecycle I propose allows us to provide this information.

Note: currently the deprecated linters are removed from presets by default (since #3405).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants