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

chore: Update go version in go.mod #371

Closed

Conversation

jmeridth
Copy link

@jmeridth jmeridth commented Feb 21, 2023

This will match go 1.19.x that is used in the Dockerfile already

Notes:

  • Why are there two require sections now?
    • Because in Go 1.17 the module graph has been changed to enable pruning and lazy loading. The second require block contains indirect dependencies
  • CI
    • Lowest chamber builds for is 1.15 here

This will match go 1.19.x that is used in the Dockerfile already

Signed-off-by: jmeridth <jmeridth@gmail.com>
@jmeridth jmeridth requested a review from a team as a code owner February 21, 2023 16:31
@jmeridth jmeridth requested review from alecjacobs5401 and removed request for a team February 21, 2023 16:31
@jmeridth
Copy link
Author

@alecjacobs5401 are you a good person to take a look at this one or is someone else better? Thank you for any insight. Cheers.

@alecjacobs5401
Copy link
Contributor

Hey @jmeridth - sorry for the late response! We've been discussing this internally for a little and are thinking through potential consequences of this change immediately.

On one hand, this is a good change and we'd like to enforce a newer go version. On the other, we are investigating the potential downstream impact to implementing libraries that might pull in chamber packages (i.e. not just using chamber as a CLI, using its internals in other go packages)

We did some looking around and it looks like mostly forks of chamber exist, but there is at least one Repository where chamber is pulled in as a package.

By changing the go version in go.mod, we do cause some potential for breaking behavior by now requiring any implementing packages to adhere to a new minimum go version.

I'm going to rope in @knksmith57 to handoff to since he is our current on-call engineer and we'll work out next steps.

@jmeridth
Copy link
Author

@alecjacobs5401 That makes sense. anytime we upgrade to a minor (especially major) version, those are the things you have to triple check. Thank you for the response. Looking forward to seeing how this pans out. Cheers.

@jmeridth
Copy link
Author

@alecjacobs5401 @knksmith57 would you prefer I closed this PR and let y'all converse and we can either reopen or create another one if decision is made to move forward? Don't want this hanging open if the timeline is a bit out. wdyt?

@knksmith57
Copy link

Hey @jmeridth I synced with the team today and I think this warrants more internal discussion based on potential blast radius.

I'm going to sync with one of our Principal's tomorrow about next steps. For now, agreed, let's close this one so it isn't floating around.

Thank you, again, for bringing this to our attention and for the PR 🙏

@jmeridth jmeridth closed this Feb 28, 2023
@jmeridth jmeridth deleted the jm/update-go-mod-go-version branch February 28, 2023 04:20
@jmeridth
Copy link
Author

@knksmith57 any chance of progress on this idea internally? Thank you in advance for any updates. Cheers.

@jmeridth
Copy link
Author

Nm. Looks like you all upgraded to 1.19 here. Nice.

@jmeridth
Copy link
Author

Solved by #383

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