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

Upgrade golangci-lint to v1.62.2 #1756

Merged
merged 4 commits into from
Dec 17, 2024
Merged

Upgrade golangci-lint to v1.62.2 #1756

merged 4 commits into from
Dec 17, 2024

Conversation

jeremybeard
Copy link
Contributor

Description

This change upgrades the golangci-lint dependency to the latest version so that it can support Go 1.23, which the CLI is also upgraded to. The corresponding linting errors are then also fixed.

📸 Screenshots

Screenshot 2024-12-12 at 12 37 01 PM

📋 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Ran make lint before taking out of draft
  • Added/updated applicable tests
  • Tested against Astro-API (if necessary).
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

@jeremybeard jeremybeard changed the title Upgrade golangci-lint Upgrade golangci-lint to v1.62.2 Dec 12, 2024
Copy link
Contributor

@neel-astro neel-astro left a comment

Choose a reason for hiding this comment

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

LGTM, left a minor nit around pkg/errors vs fmt.Errorf, but approving ahead of time,

@@ -210,8 +210,8 @@ func (d *DockerImage) Pytest(pytestFile, airflowHome, envFile, testHomeDirectory

// start pytest container
err = cmdExec(containerRuntime, stdout, stderr, []string{"start", "astro-pytest", "-a"}...)
if docErr != nil {
log.Debugf("Error starting pytest container: %s", docErr.Error())
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch 👍

@@ -68,15 +68,15 @@ func (rt DockerRuntime) initializeDocker(timeoutSeconds int) error {
// If we got an error, Docker is not running, so we attempt to start it.
_, err = rt.Engine.Start()
if err != nil {
return fmt.Errorf(dockerOpenNotice) //nolint:stylecheck
return errors.New(dockerOpenNotice)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we should stay with fmt.Errorf I think, since pkg/errors is deprecated now, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change this instance from github.com/pkg/errors to errors?

We might want a separate cleanup PR at some point to remove github.com/pkg/errors completely from the CLI, I see many instances of it, and in ways that can't be transparently switched to errors.

@jeremybeard jeremybeard mentioned this pull request Dec 17, 2024
8 tasks
@jeremybeard jeremybeard merged commit 356be0b into main Dec 17, 2024
3 checks passed
@jeremybeard jeremybeard deleted the upgrade-golangci-lint branch December 17, 2024 04:14
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