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

slogr: fix unintended API break in v1.4.0 #253

Merged
merged 2 commits into from Dec 21, 2023

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Dec 21, 2023

logr v1.3.0 had slogr.NewSlogHandler. We must keep that function for API
compatibility. https://github.com/go-logr/logr/pull/237/files#r1434203442
accidentally renamed it.

An API break in https://github.com/go-logr/logr/pull/237/files#r1434203442
wasn't noticed because the apidiff tool ran with 1.20.x.
logr v0.7.0 had slogr.NewSlogHandler. We must keep that function for API
compatibility. https://github.com/go-logr/logr/pull/237/files#r1434203442
accidentally renamed it.
@pohly
Copy link
Contributor Author

pohly commented Dec 21, 2023

Given that ToSlogHandler was in v0.8.0, do we now need to keep it also? The revised apidiff correctly warns about it: https://github.com/go-logr/logr/actions/runs/7290070078/job/19866034997?pr=253

It's cheap, so let's keep it...

@thockin
Copy link
Contributor

thockin commented Dec 21, 2023

In this case, fine. But in general, I don't think we need to be burdened with decisions we make in 0.x series tags.

@thockin thockin merged commit dcdc3f2 into go-logr:master Dec 21, 2023
14 checks passed
@pohly pohly changed the title slogr: fix unintended API break in v0.8.0 slogr: fix unintended API break in v1.4.0 Dec 21, 2023
@pohly
Copy link
Contributor Author

pohly commented Dec 21, 2023

I was confused (time for Christmas break...): logr is at 1.x, so it has strong API guarantees (no matter how new the call). 0.8.0 is the logcheck release that I just did.

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.

None yet

2 participants