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/build/devapp/owners: use owners to mark packages deprecated #38299

Open
stamblerre opened this issue Apr 8, 2020 · 2 comments
Open

x/build/devapp/owners: use owners to mark packages deprecated #38299

stamblerre opened this issue Apr 8, 2020 · 2 comments
Labels
Builders x/build issues (builders, bots, dashboards) FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@stamblerre
Copy link
Contributor

Can we use a special string to mark packages as deprecated / unsupported in the owners file? If someone sends a review for the project, we can just have GopherBot send an automated message letting the contributor know.

I was inspired by https://golang.org/cl/225297, where I almost merged a change for code that's been unsupported since 1.5. For context, I'm not familiar with most of the packages in x/tools, but I check GitHub PRs biweekly to make sure they get assigned reviewers (due to #31658). This change looked simple enough to merge, and I only didn't because I decided to attempt to figure out who the owner for that package was (the lack of owners for x/tools is a whole separate issue I hope to tackle one day...).

@gopherbot gopherbot added this to the Unreleased milestone Apr 8, 2020
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Apr 8, 2020
@andybons andybons changed the title x/build: use owners to mark packages deprecated x/build/devapp/owners: use owners to mark packages deprecated Apr 8, 2020
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 8, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Apr 10, 2020

Can we use a special string to mark packages as deprecated / unsupported in the owners file?

Can you elaborate on why a special string is needed? What are you referring to by "the owners file", is it the https://dev.golang.org/owners page?

There's already an official convention for marking Go packages as deprecated, see https://golang.org/wiki/Deprecated. (There's also another convention for marking packages as frozen, which is related but not the same.)

There exists code to detect when a package is marked as being deprecated using that convention. E.g., staticcheck implements it.

Do you think think that would be sufficient?

I think it would be helpful to maintain whether or not a package is deprecated in one place, not more.

I was inspired by https://golang.org/cl/225297, where I almost merged a change for code that's been unsupported since 1.5.

If I understand the motivation in this issue correctly, it sounds like the following changes could be investigated and considered in order to improve things.

  1. The package golang.org/x/tools/cmd/cover should be marked as deprecated in favor of cmd/cover.

  2. When a CL is sent that modifies a package that is known to be deprecated, there can be a warning sent.

  3. Package list at https://dev.golang.org/owner should either not list deprecated packages as having owners, or make it clear if a package is deprecated.

Does this match what you had in mind? I'm asking this to understand the issue better.

Step 1 is trivial, but 2 and 3 should be discussed more.

@stamblerre
Copy link
Contributor Author

I agree that we should probably mark deprecated packages correctly - thanks for pointing me to that link.

That may be sufficient in this case, but as a general idea, it might be nice to have some auto-reply that tells users that they sent a change for a deprecated package (if they missed the docs or don't have staticcheck enabled). The reason I suggested the special string is just because it sounded like a fairly easy thing to do (and yep, I'm referring to https://dev.golang.org/owners).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants