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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃尡 Bump golangci-lint to v1.46.2. #1157

Merged
merged 1 commit into from Aug 17, 2022
Merged

Conversation

s3rj1k
Copy link
Member

@s3rj1k s3rj1k commented Aug 5, 2022

Linter bump is needed to support building BMO with golang 1.18, as older version of linter fails to lint when default version of golang compiler is 1.18

(v1.46.2 version is chosen as most stable version that works with 1.17 and 1.18)

Signed-off-by: s3rj1k evasive.gyron@gmail.com

@metal3-io-bot metal3-io-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 5, 2022
@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 5, 2022

/test unit

Signed-off-by: s3rj1k <evasive.gyron@gmail.com>
@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 5, 2022

/test-ubuntu-integration-main
/test-centos-integration-main

@honza
Copy link
Member

honza commented Aug 5, 2022

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2022
Copy link
Member

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

I think this change is overlapping with https://github.com/metal3-io/baremetal-operator/pull/1149/files

@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 8, 2022

I think this change is overlapping with https://github.com/metal3-io/baremetal-operator/pull/1149/files

That change bumps linter to v1.47.1 and it's a bad idea, that version has some broken revive linters
Not sure if they are used in BMO.

Still current change can be merged right now and enables to build BMO with golang 1.18 and does not touch k8s libraries.

I would prefer to get this landed before that big change, and get a tag release before that big k8s library bump lands.

I have issues with vendoring newer k8s API that are present in https://github.com/metal3-io/baremetal-operator/pull/1149/files

@furkatgofurov7
Copy link
Member

furkatgofurov7 commented Aug 10, 2022

That change bumps linter to v1.47.1 and it's a bad idea, that version has some broken revive linters
Not sure if they are used in BMO.

@s3rj1k I have not seen any broken linters so far in #1149, and also I am not sure why it can be a bad idea to use that version (which should work fine with Go 1.18) and rather stick to v1.46.X.

@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 10, 2022

@furkatgofurov7 golangci/golangci-lint#2999

I assume that this issue will be fixed only in golangci-lint v1.48.1

@furkatgofurov7
Copy link
Member

I assume that this issue will be fixed only in golangci-lint v1.48.1

Ack, v1.48.0 was released some time ago, and we could wait for the v1.48.1 and bump it in this PR or 1149 also, please let me know which works for you best.

@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 15, 2022

@furkatgofurov7 v1.48.1 would be preferable, thanks

@furkatgofurov7
Copy link
Member

@furkatgofurov7 v1.48.1 would be preferable, thanks

sure, would you update this PR once it is out or want me to do it as part of #1149 ?

@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 15, 2022

sure, would you update this PR once it is out or want me to do it as part of #1149 ?

I'll update this one, I hope to have one tag release before K8s libs are bumped

@dtantsur
Copy link
Member

/approve

@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2022
@metal3-io-bot metal3-io-bot merged commit 6076e74 into metal3-io:main Aug 17, 2022
@s3rj1k
Copy link
Member Author

s3rj1k commented Aug 17, 2022

@dtantsur Thanks

@furkatgofurov7 furkatgofurov7 changed the title Bump golangci-lint to v1.46.2. 馃尡 Bump golangci-lint to v1.46.2. Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants