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

Update controller-runtime to v0.9.2 #267

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

saschagrunert
Copy link
Contributor

@saschagrunert saschagrunert commented Jun 8, 2021

Description of your changes

This patch updates the controller-runtime dependency to the latest release.

Closes #265

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

@saschagrunert
Copy link
Contributor Author

CI is green 💚

Copy link
Member

@negz negz left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, and I don't see anything too concerning in the v0.9.0 release notes.

https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.9.0

GO_VERSION: '1.14'
GOLANGCI_VERSION: 'v1.31'
GO_VERSION: '1.16'
GOLANGCI_VERSION: 'v1.40.1'
Copy link
Member

Choose a reason for hiding this comment

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

Is this change necessary? I generally like the idea of bumping the version of golangci-lint we're running, but unfortunately we actually run it multiple times during a build. One is triggered by make lint and uses the version in the build submodule, which is v1.31. The other uses this GitHub Action. We run both because we like that the GitHub Action leaves comments on PRs with linter violations (and because I haven't got around to disabling make lint being called as part of the other CI jobs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the only way to make the CI happy. 🤔 Do you prefer to downgrade the linter?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally we'd bump to Go 1.16 but keep using golangci-lint 1.31. Can you provide some context on how that combination breaks CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Anything we can do about this? (bumping the version fixes the issue)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted my change to fixup the CI again.

Copy link
Member

Choose a reason for hiding this comment

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

@saschagrunert I believe I had similar issues with CI in my PR, and able to work around with: crossplane-contrib/provider-helm@90bb4b3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, fixed as suggested and the CI seems green now.

@saschagrunert saschagrunert force-pushed the controller-runtime branch 2 times, most recently from b9898e4 to 7849b18 Compare June 29, 2021 13:43
@saschagrunert saschagrunert changed the title Update controller-runtime to v0.9.0 Update controller-runtime to v0.9.2 Jun 29, 2021
@saschagrunert saschagrunert force-pushed the controller-runtime branch 2 times, most recently from 7971b1f to b16ce80 Compare June 29, 2021 13:45
@turkenh
Copy link
Member

turkenh commented Jul 11, 2021

@saschagrunert thanks a lot for working on this. I need this in provider-helm to bump the helm version.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
This patch updates the controller-runtime dependency to the latest
release.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@negz negz merged commit 85b19c2 into crossplane:master Jul 13, 2021
@saschagrunert saschagrunert deleted the controller-runtime branch July 14, 2021 06:59
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