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

cleanup golagci-lint configuration #5482

Merged
merged 1 commit into from
May 9, 2024

Conversation

lburgazzoli
Copy link
Contributor

@lburgazzoli lburgazzoli commented May 8, 2024

This PR aims to reduce the number of excluded lints, ideally we should revisit the usefulness of the disabled ones and re-enable them.

@lburgazzoli lburgazzoli changed the title chore: cleanup golagci-lint configuration cleanup golagci-lint configuration May 8, 2024
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, we can try to remove the disable at all and see how many failures we have. If they are too much, we can create a gh issue with a task for each disabled lint in order to enable one at a time. This kind of chore tasks should require more attention, thanks for bringing it up.

@lburgazzoli
Copy link
Contributor Author

IMO, we can try to remove the disable at all and see how many failures we have. If they are too much, we can create a gh issue with a task for each disabled lint in order to enable one at a time. This kind of chore tasks should require more attention, thanks for bringing it up.

I did test every single exclusion by one but to fix the findings, I would have had to amend/refactor a very large set of files hence I opted to be conservative and just re-enable those that have no side effects.

I will probably start enabling additional linters one by one as follow up PRs as soon as I have time

@lburgazzoli lburgazzoli merged commit fe87df3 into apache:main May 9, 2024
12 checks passed
@lburgazzoli lburgazzoli deleted the lint-cleanup branch May 9, 2024 08:54
@squakez
Copy link
Contributor

squakez commented May 9, 2024

I will probably start enabling additional linters one by one as follow up PRs as soon as I have time

Feel free to add an issue tracker as I suggested, so we can all contribute to the same activity when we have some free time.

@lburgazzoli
Copy link
Contributor Author

#5486

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.

3 participants