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

chore: Deprecate legacy ID-based team methods #3373

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

stevehipwell
Copy link
Contributor

Resolves #3368

This PR makes all legacy ID based team methods as deprecated based on the closing down notice.

@gmlewis gmlewis changed the title chore: Deprecated legacy ID based team methods chore: Deprecate legacy ID based team methods Dec 9, 2024
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.30%. Comparing base (2b8c7fa) to head (ff412f6).
Report is 190 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3373      +/-   ##
==========================================
- Coverage   97.72%   92.30%   -5.42%     
==========================================
  Files         153      176      +23     
  Lines       13390    15031    +1641     
==========================================
+ Hits        13085    13874     +789     
- Misses        215     1064     +849     
- Partials       90       93       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stevehipwell
Copy link
Contributor Author

@gmlewis the metadata generation doesn't look right. I manually updated all of the metadata (before I knew about the script) to fix the incorrect docs and URL templates for the legacy API calls but generate is putting them back.

Just a nit but is there a reason why //meta: isn't // meta:?

@gmlewis
Copy link
Collaborator

gmlewis commented Dec 9, 2024

Just a nit but is there a reason why //meta: isn't // meta:?

Yes, that is an indication to the reader and the tools that this comment is used by the tooling.

@stevehipwell
Copy link
Contributor Author

@gmlewis but what about the issue with linking to the incorrect docs?

https://github.com/google/go-github/blob/master/github/teams.go#L107...L112

@gmlewis
Copy link
Collaborator

gmlewis commented Dec 9, 2024

@gmlewis but what about the issue with linking to the incorrect docs?

The links are managed by the official GitHub OpenAPI specification, and it periodically needs to be updated (which was last done here: #3352 ). I have not done a deep dive into this issue yet, but it may need updating again to fix the issue.

@gmlewis gmlewis mentioned this pull request Dec 9, 2024
@gmlewis
Copy link
Collaborator

gmlewis commented Dec 9, 2024

I've gone ahead and created #3374. I'm not sure if that will help your PR yet or not.

@gmlewis
Copy link
Collaborator

gmlewis commented Dec 9, 2024

Once it is merged, you can update your local master branch with it, then merge master into this PR to see if it fixes the problems you are seeing.

@stevehipwell
Copy link
Contributor Author

@gmlewis I don't think #3374 will help as the docs that should be linked are already present (search for -legacy at the bottom of the diff).

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@stevehipwell stevehipwell force-pushed the deprecate-team-id-methods branch from 155894b to ff412f6 Compare December 9, 2024 16:23
@gmlewis gmlewis changed the title chore: Deprecate legacy ID based team methods chore: Deprecate legacy ID-based team methods Dec 9, 2024
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @stevehipwell !
LGTM.
Merging.

@gmlewis gmlewis merged commit 0162c9d into google:master Dec 9, 2024
6 of 7 checks passed
@stevehipwell stevehipwell deleted the deprecate-team-id-methods branch December 10, 2024 13:04
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.

Mark teams functionality that's based on ID as deprecated
2 participants