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

Updates for Go 1.17 #7

Merged
merged 4 commits into from
Sep 10, 2021
Merged

Updates for Go 1.17 #7

merged 4 commits into from
Sep 10, 2021

Conversation

ChrisHines
Copy link
Member

@ChrisHines ChrisHines commented Aug 20, 2021

I want to update this package and tag a patch release before updating go-kit/kit in go-kit/kit#1105. I've already updated both of this package's dependencies along the same lines. Just working my way up the tree to minimize the number of new releases and maximize the benefit of the Go 1.17 module pruning feature for consumers.

@ChrisHines ChrisHines changed the title Enable go 1.17 module optimizations and update dependencies Updates for Go 1.17 Aug 20, 2021
@ChrisHines
Copy link
Member Author

@peterbourgon ping.

@ChrisHines ChrisHines merged commit ad0641e into main Sep 10, 2021
@ChrisHines ChrisHines deleted the go1.17 branch September 10, 2021 05:17
@sagikazarmark
Copy link
Contributor

Should we tag this change, so we can make the ones in the kit repo?

@peterbourgon
Copy link
Member

@sagikazarmark Can you clarify? I see this as part of the next milestone...

@ChrisHines
Copy link
Member Author

This package still has a dependency on github.com/go-stack/stack that is only used in tests. We could probably find a way to remove that dependency and therefore drop it from the go.mod file. If that is worth doing then it makes sense to hold off on tagging a release until we had that change in place.

@sagikazarmark
Copy link
Contributor

@peterbourgon My understanding is that tagging the log package blocks advancing to the Go 1.17 change in the kit repo. At least that was my assumption based on Chris's first comment.

@ChrisHines let me take a look at dropping the stack dependency.

@ChrisHines
Copy link
Member Author

It's not so much blocking the Go 1.17 change in the kit repo, but without it some the module graph pruning behavior of Go 1.17 would not apply to the log package. So the optimal sequence is to tag log first, then update the go.mod file in kit and tag kit.

@sagikazarmark
Copy link
Contributor

I think we are ready for that, aren't we? I managed to get rid of the stack package.

@ChrisHines
Copy link
Member Author

I saw your PR. I'll review it later when I have a block of time. I expect to make a couple of comments to see if we can simplify.

@peterbourgon
Copy link
Member

We can tag a new release, sure. Does the bump to 1.17 warrant 0.2.0 or can we get away with 0.1.1?

@sagikazarmark
Copy link
Contributor

@ChrisHines it's already merged, but I'm happy to followup.

@peterbourgon My default answer would be 0.2.0, but I have no strong opinion on the matter.

@ChrisHines
Copy link
Member Author

@sagikazarmark Ahh, in that case I might tinker with it on my own tonight and see if I find any improvements. In any case, thanks for making the change. :)

Regarding version. I did a patch bump on some other packages, but only the go version in the go.mod changed in those cases. In this case, since we're removing a dependency, I think v0.2.0 seems slightly more appropriate.

@peterbourgon
Copy link
Member

I'll do that in a couple hours unless there's an objection. Should it be immediately followed by any action in kit?

@sagikazarmark
Copy link
Contributor

No objection. Chris's followup PR was also merged, so I think we are good.

The next action in kit would be updating the log module in go-kit/kit#1105 and finishing up the PR for merging.

@ChrisHines
Copy link
Member Author

Yep, we should be good to tag this repo.

@peterbourgon
Copy link
Member

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