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

chore: enable golangci-lint on repository #138

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

ccoVeille
Copy link
Contributor

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Apr 10, 2024

The tests will fail until #132 will be merged.

This way, it will give you an idea of the errors it might report.

@ccoVeille ccoVeille force-pushed the add-golangci-action branch 2 times, most recently from f99bd7c to 2dc9d0f Compare April 10, 2024 20:04
@ccoVeille ccoVeille force-pushed the add-golangci-action branch from 2dc9d0f to c06089b Compare April 11, 2024 14:44
@ccoVeille
Copy link
Contributor Author

pushed back after a rebase, all tests should pass, let see

@kehoecj kehoecj added the OSS Community Contribution Contributions from the OSS Community label Apr 11, 2024
@ccoVeille
Copy link
Contributor Author

ccoVeille commented Apr 12, 2024

I suggest to merge it before any other PR, so the tests would be run on new PR, ensuring I hope maintaining code quality.

Do not hesitate to ping me for help on any PR where you or contributor will face golangci-lint issues.

But golangci-lint guidelines and documentation is great.

https://golangci-lint.run/

Please note that you only have to install golangci-lint 1.57.2 or newer locally and run the following command to replicate locally the GitHub action I installed

golangci-lint run ./...

@ccoVeille
Copy link
Contributor Author

Is everything alright with this PR?

I mean tell me if you need something from me.

@kehoecj kehoecj self-requested a review April 16, 2024 13:40
Copy link
Collaborator

@kehoecj kehoecj left a comment

Choose a reason for hiding this comment

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

LGTM - a few questions

@kehoecj kehoecj added the pr-action-requested PR is awaiting feedback from the submitting developer label Apr 16, 2024
@kehoecj kehoecj merged commit c8df506 into Boeing:main Apr 16, 2024
7 checks passed
@kehoecj
Copy link
Collaborator

kehoecj commented Apr 16, 2024

Big thanks @ccoVeille for adding golangci-lint functionality to the pipeline!

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Apr 16, 2024

You won't thank me when it will report errors on every PR 😅

Anyway, golangci-lint was a great discovery for me. It helped me to find many bugs in my code. They keep adding linters.

I recommend you to read golangci-lint website and especially the false positive section.

@kehoecj
Copy link
Collaborator

kehoecj commented Apr 16, 2024

@ccoVeille Any idea why the linter on windows is failing? https://github.com/Boeing/config-file-validator/actions/runs/8709768011/job/23890548496

@nickajacks1
Copy link
Contributor

@kehoecj I couldn't reproduce the problem on my windows PC but this error is sometimes fixed by doing go mod tidy

@ccoVeille
Copy link
Contributor Author

@ccoVeille Any idea why the linter on windows is failing? https://github.com/Boeing/config-file-validator/actions/runs/8709768011/job/23890548496

The message is about timeout parameter

It can be increased in .golangci.yaml

https://golangci-lint.run/usage/configuration/#run-configuration

The default value (1min) might be too short for windows docker instance, try 5m

shiina4119 pushed a commit to shiina4119/config-file-validator that referenced this pull request Aug 23, 2024
shiina4119 pushed a commit to shiina4119/config-file-validator that referenced this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OSS Community Contribution Contributions from the OSS Community pr-action-requested PR is awaiting feedback from the submitting developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable golangci-lint on repository
3 participants