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

misc: update golangci-lint and use cached action #35

Merged
merged 4 commits into from
Oct 4, 2024

Conversation

thor
Copy link

@thor thor commented Oct 3, 2024

Disable depguard as it's unconfigured and ignore the noop logger for unused parameters.

@thor thor changed the base branch from main to hanst/upgrade-various-deps October 3, 2024 12:28
@thor thor force-pushed the misc/update-golangci-lint branch from 6a74ba8 to db6b7a2 Compare October 3, 2024 12:38
@thor thor requested a review from hanssto October 3, 2024 13:38
@thor thor marked this pull request as ready for review October 3, 2024 13:38
@thor
Copy link
Author

thor commented Oct 3, 2024

Do you want to merge the other PR as-is, @hanssto, and then we can rebase and merge this one? It looks like it'd be nice to have the full history for these smaller changes.

@hanssto
Copy link

hanssto commented Oct 3, 2024

I'm torn between "I shouldn't spend more time on this" and "I should rework the other PR so that it's more sane in the event of us ever merging this back upstream". I will let tomorrow-me decide as I have to pick up the spawn.

@thor
Copy link
Author

thor commented Oct 3, 2024

I'm torn between "I shouldn't spend more time on this" and "I should rework the other PR so that it's more sane in the event of us ever merging this back upstream". I will let tomorrow-me decide as I have to pick up the spawn.

My suggestion is, @hanssto:

  1. Merge the base branch of this PR as a squash as all the changes are the same in nature.
  2. Merge this via rebase and merge to the main branch.

Au voila.

hanssto added a commit that referenced this pull request Oct 4, 2024
We have a critical security update on Docker auth, which is an
indirect dependency of the test code (seemingly via oras).

This is the culmination of different attempts at upgrading without
having to make substantial changes to the sloth source code.

Some notes:
* Upgrading go-helm-template (also made by slok) in test ultimately
  requires at least Go 1.22.
* The GH actions uses the root go.mod Go version, so these need
  to be synced.
* The next version of reloader, also made by slok, requires
  1.23, so both root and test needs to be on 1.23.
* Updated the dev and prod Dockerfiles to match the go version.
* Upgraded most dependencies to get as far as possible.
* Keeping Prometheus-related deps as certain newer versions
  don't work with the code. There might be further version jumps
  we can do here.

This results in errors on lint which are addressed in #35.
@hanssto hanssto force-pushed the hanst/upgrade-various-deps branch from e1c5f6e to 776c994 Compare October 4, 2024 10:14
@hanssto
Copy link

hanssto commented Oct 4, 2024

@thor I've cleaned up #34 now, squashing the commits manually and giving them a more unified explanation. Lemme know if it's readable.

@thor
Copy link
Author

thor commented Oct 4, 2024

@thor I've cleaned up #34 now, squashing the commits manually and giving them a more unified explanation. Lemme know if it's readable.

@hanssto Looks like a high-quality commit message to me. 🙌 Let's rebase and merge this into #34, then rebase and merge it to main?

hanssto added a commit that referenced this pull request Oct 4, 2024
We have a critical security update on Docker auth, which is an
indirect dependency of the test code (seemingly via oras).

This is the culmination of different attempts at upgrading without
having to make substantial changes to the sloth source code.

Some notes:
* Upgrading go-helm-template (also made by slok) in test ultimately
  requires at least Go 1.22.
* The GH actions uses the root go.mod Go version, so these need
  to be synced.
* The next version of reloader, also made by slok, requires
  1.23, so both root and test needs to be on 1.23.
* Updated the dev and prod Dockerfiles to match the go version.
* Upgraded most dependencies to get as far as possible.
* Keeping Prometheus-related deps as certain newer versions
  don't work with the code. There might be further version jumps
  we can do here.

This results in errors on lint which are addressed in #35.
Base automatically changed from hanst/upgrade-various-deps to main October 4, 2024 10:52
@hanssto hanssto force-pushed the misc/update-golangci-lint branch from 21ce7a8 to fba8e97 Compare October 4, 2024 10:57
Copy link

@hanssto hanssto left a comment

Choose a reason for hiding this comment

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

LGTM, and so green.

@hanssto hanssto merged commit 8f417f6 into main Oct 4, 2024
36 checks passed
@hanssto hanssto deleted the misc/update-golangci-lint branch October 4, 2024 11:10
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.

2 participants