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.

Sign up for GitHub

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

Jump to bottom

x/tools/go/{packages/packagestest,expect}: deprecate, tag, and delete #70229

Open
adonovan opened this issue Nov 6, 2024 · 8 comments
Open

x/tools/go/{packages/packagestest,expect}: deprecate, tag, and delete #70229

adonovan opened this issue Nov 6, 2024 · 8 comments
Assignees
@adonovan
Labels
Proposal-Accepted Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@adonovan
Copy link
Member

adonovan commented Nov 6, 2024 •
edited
Loading

Background: These packages, designed for use in tests in x/tools, were published without proper deliberation, so any changes to their public APIs must go through the proposal process. There are dozens of improvements we (in x/tools) would like to have made that are simply too costly as a result. However, these packages are almost never used outside x/tools. Of the couple dozen imports reported by pkg.go.dev, nearly all are in repos that are forks of x/tools. The only real one is is k8s.io.

Proposal: We propose to fork these packages to an internal subtree, and then to tag and delete the public packages using the same process as #59676.
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Nov 6, 2024
@gopherbot gopherbot added this to the Unreleased milestone Nov 6, 2024
@adonovan adonovan moved this to Incoming in Proposals Nov 6, 2024
@adonovan adonovan self-assigned this Nov 6, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/625919 mentions this issue: x/tools: use internal/expect instead of go/expect

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/625918 mentions this issue: internal/expect: fork go/expect

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/625920 mentions this issue: internal/packagestest: fork go/packages/packagestest

@adonovan adonovan changed the title x/tools/go/{packages/packagestest,expect}: tag and delete x/tools/go/{packages/packagestest,expect}: deprecate, tag, and delete Nov 6, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/625921 mentions this issue: go/{expect,packages/packagestest}: mention the tag+delete proposal

gopherbot pushed a commit to golang/tools that referenced this issue Nov 6, 2024
@adonovan @gopherbot
internal/expect: fork go/expect
b22f1ad 
Almost no-one outside x/tools uses it, so we'd like to evolve
it for our needs, and tag and delete the public package.

Updates golang/go#70229

Change-Id: I77c7923881efdf772a1ad53134483ad0078c941d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625918
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
gopherbot pushed a commit to golang/tools that referenced this issue Nov 6, 2024
@adonovan @gopherbot
x/tools: use internal/expect instead of go/expect
ca2b41b 
The only remaining uses of go/expect are from packagestest.

Updates golang/go#70229

Change-Id: I5a8c835b761381747fbd3f936d261ed773b536e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625919
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@findleyr
Copy link
Member

findleyr commented Nov 6, 2024

This seems like an easy decision to me. Essentially nobody is using these packages, and yet they have a large and restrictive API surface area, which is tightly coupled to our tests. Moving these to internal will reduce friction at essentially no cost.

I'd argue that if folks want to use these packages, they are better off forking in any case, for the same reasons.

gopherbot pushed a commit to golang/tools that referenced this issue Nov 6, 2024
@adonovan
internal/packagestest: fork go/packages/packagestest
efcd2bd 
Also, update all imports outside of that package.

Updates golang/go#70229

Change-Id: I8b08f892ec86d560c0406319c2954eb9912d78ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625920
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Nov 6, 2024
@adonovan
go/{expect,packages/packagestest}: mention the tag+delete proposal
0b9e499 
Updates golang/go#70229

Change-Id: I831fe290b8e5ec9d1010e7b9d569543e5c5b2cec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625921
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@dmitshur dmitshur mentioned this issue Dec 2, 2024
Closed
dennypenta pushed a commit to dennypenta/tools that referenced this issue Dec 3, 2024
@adonovan @dennypenta
internal/expect: fork go/expect
8eae06e 
Almost no-one outside x/tools uses it, so we'd like to evolve
it for our needs, and tag and delete the public package.

Updates golang/go#70229

Change-Id: I77c7923881efdf772a1ad53134483ad0078c941d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625918
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
dennypenta pushed a commit to dennypenta/tools that referenced this issue Dec 3, 2024
@adonovan @dennypenta
x/tools: use internal/expect instead of go/expect
93437db 
The only remaining uses of go/expect are from packagestest.

Updates golang/go#70229

Change-Id: I5a8c835b761381747fbd3f936d261ed773b536e3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625919
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
dennypenta pushed a commit to dennypenta/tools that referenced this issue Dec 3, 2024
@adonovan @dennypenta
internal/packagestest: fork go/packages/packagestest
f26a5f9 
Also, update all imports outside of that package.

Updates golang/go#70229

Change-Id: I8b08f892ec86d560c0406319c2954eb9912d78ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625920
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
dennypenta pushed a commit to dennypenta/tools that referenced this issue Dec 3, 2024
@adonovan @dennypenta
go/{expect,packages/packagestest}: mention the tag+delete proposal
128c28a 
Updates golang/go#70229

Change-Id: I831fe290b8e5ec9d1010e7b9d569543e5c5b2cec
Reviewed-on: https://go-review.googlesource.com/c/tools/+/625921
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@rsc rsc moved this from Incoming to Active in Proposals Dec 4, 2024
@rsc
Copy link
Contributor

rsc commented Dec 4, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@aclements
Copy link
Member

Based on the discussion above, this proposal seems like a likely accept.

The proposal is to fork the golang.org/x/tools/go/packages/packagestest and golang.org/x/tools/go/expect packages to an internal subtree, and then to tag and delete the public packages using the same process as #59676.

@aclements aclements moved this from Active to Likely Accept in Proposals Jan 8, 2025
@aclements aclements moved this from Likely Accept to Accepted in Proposals Jan 16, 2025
@aclements
Copy link
Member

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.

The proposal is to fork the golang.org/x/tools/go/packages/packagestest and golang.org/x/tools/go/expect packages to an internal subtree, and then to tag and delete the public packages using the same process as #59676.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adonovan adonovan

Labels
Proposal-Accepted Tools This label describes issues relating to any tools in the x/tools repository.
Projects
Proposals
Status: Accepted
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@rsc @aclements @adonovan @gopherbot @findleyr