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

[WIP] replaced go-kit/kit with go-kit/log #302

Closed
wants to merge 3 commits into from
Closed

[WIP] replaced go-kit/kit with go-kit/log #302

wants to merge 3 commits into from

Conversation

RinkiyaKeDad
Copy link
Contributor

Signed-off-by: RinkiyaKeDad arshsharma461@gmail.com

github.com/go-kit/kit is only being used for the log packages and brings in a lot of dependencies as compared to github.com/go-kit/log. This would help determine how useful switching would be.

For reference see kubernetes/kubernetes#102144 and this discussion.

cc @dims @gautierdelorme

Signed-off-by: RinkiyaKeDad <arshsharma461@gmail.com>
Signed-off-by: RinkiyaKeDad <arshsharma461@gmail.com>
Signed-off-by: RinkiyaKeDad <arshsharma461@gmail.com>
@roidelapluie
Copy link
Member

Thank you. We have no plans to switch to github.com/go-kit/log as long as it is alpha release. We are expecting Peter to ping us when it is ready.

@dims
Copy link

dims commented May 28, 2021

@roidelapluie hence the WIP. We can't move k/k to newer prometheus common until this happens. So we are all in the wait mode.

We do have some time to switch over ( see 1.22 release schedule https://www.kubernetes.dev/resources/release/ for code freeze ) but would like to get this into all our CI jobs etc way in advance to avoid any nasty surprises.

Yes, we would love to have a prometheus/common tagged for us to use as well when the time comes.

Thanks for all your help!

/cc @peterbourgon @gouthamve

PS: https://github.com/kubernetes/kubernetes/pull/102144/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6L78 (we are currently at v0.10.0 of prometheus/common)

@peterbourgon
Copy link

peterbourgon commented May 28, 2021

I don't see how I could have been any clearer about this:

Screen Shot 2021-05-28 at 1 15 56 PM

We will do a proper release in the short term. Until then, absolutely do not merge this PR.

@dims
Copy link

dims commented May 28, 2021

@peterbourgon yes of course, this was open as a work-in-progress to check if there was anything else needed. We definitely don't want an alpha tag to get into kubernetes go.mod if we can avoid it.

@peterbourgon
Copy link

@dims OK, cool — put it in Draft state, then, probably?

@dims
Copy link

dims commented May 28, 2021

@peterbourgon that's totally fair. good point.

@roidelapluie
Copy link
Member

roidelapluie commented May 28, 2021

I prefer to close this pull request, so that we do not depend on you being around when we need to do the switch (by then the pull request would need to be re-done anyway).

Thank you anyway. As said, we are following the situation and will upgrade when needed.

@sagikazarmark
Copy link
Contributor

The library is now marked as stable.

@dims
Copy link

dims commented Jun 2, 2021

@roidelapluie @peterbourgon let me know if you want me to file the PR here ( against https://github.com/go-kit/log/tree/v0.1.0 )

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.

5 participants