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

x/exp/apidiff: ignore "internal" packages during comparison #52864

Closed
pohly opened this issue May 12, 2022 · 4 comments
Closed

x/exp/apidiff: ignore "internal" packages during comparison #52864

pohly opened this issue May 12, 2022 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@pohly
Copy link

pohly commented May 12, 2022

What version of Go are you using (go version)?

$ go version
1.13 (only for building)

Does this issue reproduce with the latest release?

Yes (of apidiff).

What did you do?

We use apidiff in a Go project to prevent merging of PRs which break the API:
https://github.com/kubernetes/klog/blob/main/hack/verify-apidiff.sh

One PR removes an exported function from "internal/serialize": kubernetes/klog#326

What did you expect to see?

apidiff should not complain.

What did you see instead?

https://github.com/kubernetes/klog/runs/6372237998?check_suite_focus=true

Inspecting API of ../old...
go: downloading github.com/go-logr/logr v1.2.0
go: extracting github.com/go-logr/logr v1.2.0
go: finding github.com/go-logr/logr v1.2.0
Comparing with ../old...
FAIL: k8s.io/klog/v2/internal/serialize contains incompatible changes:
- TrimDuplicates: removed
@heschi
Copy link
Contributor

heschi commented May 12, 2022

cc @jba

@heschi heschi added the NeedsFix The path to resolution is known, but the work has not been done. label May 12, 2022
@heschi heschi added this to the Unreleased milestone May 12, 2022
@seankhliao seankhliao changed the title apidiff: ignore "internal" packages during comparison x/exp/apidiff: ignore "internal" packages during comparison May 12, 2022
@gopherbot
Copy link

Change https://go.dev/cl/406534 mentions this issue: apidiff: ignore internal pack

@jba
Copy link
Contributor

jba commented May 16, 2022

@pohly, could you look at the above CL and see if it meets your needs?

I feel that it shouldn't be impossible to compare internal packages. I think a flag would help.

@pohly
Copy link
Author

pohly commented May 17, 2022

That change made it work for us without any changes on our side, thanks!

I agree that a flag may be useful for other scenarios, but disabled by default seems like the right choice.

@golang golang locked and limited conversation to collaborators May 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants