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

CLOUDP-186801: disable depguard linter #1014

Closed
wants to merge 1 commit into from

Conversation

josvazg
Copy link
Collaborator

@josvazg josvazg commented Jun 27, 2023

All Submissions:

  • Have you signed our CLA?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if there is one).
  • Update docs/release-notes/release-notes.md if your changes should be included in the release notes for the next release.

@helderjs
Copy link
Collaborator

@josvazg We should not disable the depguard linter, but update golangci-lint in the project and adjust its config.

@josvazg
Copy link
Collaborator Author

josvazg commented Jun 27, 2023

@josvazg We should not disable the depguard linter, but update golangci-lint in the project and adjust its config.

This change is to make the development environment make lint behaviour match the current CI state.

What is the value of having development fail for something the CI is not checking yet?
We can enable it back on the same PR that fixes the config and would make the CI use depguard.

@helderjs
Copy link
Collaborator

@josvazg Why do you think CI is not checking it?
The latest depguard version has changed its default behavior, requiring to explicitly setting a config for it.
OpenPeeDeeP/depguard#55
OpenPeeDeeP/depguard#49

@josvazg
Copy link
Collaborator Author

josvazg commented Jun 27, 2023

It seemed so, because make lint in development fails with a lot of depguard failures. In any case, the point of this PR is to fix the development behavior by default, while the proper fix gets deployed to both dev and the CI.

@josvazg
Copy link
Collaborator Author

josvazg commented Jun 27, 2023

If I understood correctly, I could alternatively just drop this PR and instead keep on using the same linter version as the CI to have the same results.
I will be closing this then.
Thanks!

@josvazg josvazg closed this Jun 27, 2023
@josvazg josvazg deleted the CLOUDP-186801/depguard-off branch June 27, 2023 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants