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

ci: stablize golangci-lint task #1007

Merged
merged 3 commits into from
Apr 10, 2023
Merged

ci: stablize golangci-lint task #1007

merged 3 commits into from
Apr 10, 2023

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Apr 10, 2023

This closes #1005.

Modifications

  1. Replace the usage of golangci-lint action with barely bin commands.
  2. Support run with the specified version with one command make lint and store the binutil project-wise.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / GoDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun mentioned this pull request Apr 10, 2023
1 task
@shibd
Copy link
Member

shibd commented Apr 10, 2023

@tisonkun Please help explain why?

@tisonkun
Copy link
Member Author

why

Why for what?

You can refer to the issue for the failure. Generally, we use a v3 tag for golangci-lint action which is not fixed. So anything can be changed from upstream. This patch pinned the golangci-lint to v1.51.2 and does not use the action to ignore any bad case there.

@shibd
Copy link
Member

shibd commented Apr 10, 2023

why

Why for what?

You can refer to the issue for the failure. Generally, we use a v3 tag for golangci-lint action which is not fixed. So anything can be changed from upstream. This patch pinned the golangci-lint to v1.51.2 and does not use the action to ignore any bad case there.

FYI, we have fixed the version here.

https://github.com/apache/pulsar-client-go/pull/951/files

@tisonkun
Copy link
Member Author

@shibd Yep. So the problem is that we are using an action that can produce some unpredictable issues, also we can hardly run it locally - said other projects use a different version but either of them installed in $GOBIN/golangci-lint.

@tisonkun
Copy link
Member Author

Merging...

Unblock other PRs.

@tisonkun tisonkun merged commit 16b1fa3 into apache:master Apr 10, 2023
@tisonkun tisonkun deleted the ci branch April 10, 2023 15:34
@Gleiphir2769
Copy link
Contributor

Gleiphir2769 commented Apr 11, 2023

Good work! @tisonkun

But I am very curious why httpclient can pass the previous CI. It has already been introduced in 2021. The old golangci-lint we used has the ability for ignoring old files?

I do not find any golangci-lint doc for this so I can not understand why this happens. This is so weird.

@BewareMyPower BewareMyPower added this to the v0.11.0 milestone Apr 11, 2023
@tisonkun
Copy link
Member Author

But I am very curious why httpclient can pass the previous CI. It has already been introduced in 2021. The old golangci-lint we used has the ability for ignoring old files?

I bump the golangci-lint to 1.51.2 because original 1.50.1 doesn't work locally for unknown reason - I don't spend time to figure it out but simply do a small version bump.

Besides, it's a false positive on bodyclose linter. There are a few false positive reports upstream, like timakin/bodyclose#42.

@Gleiphir2769
Copy link
Contributor

Besides, it's a false positive on bodyclose linter. There are a few false positive reports upstream, like timakin/bodyclose#42.

I see. It's actually a false positive. Thanks!

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.

[CI] Golangci-lint always failed and block the master branch
4 participants