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

Support new Git reference get/list endpoints #1512

Closed
bluekeyes opened this issue Apr 29, 2020 · 2 comments · Fixed by #1513
Closed

Support new Git reference get/list endpoints #1512

bluekeyes opened this issue Apr 29, 2020 · 2 comments · Fixed by #1513

Comments

@bluekeyes
Copy link
Contributor

Sometime in 20191, GitHub quietly hid the old /repos/:owner/:repo/git/refs[/...] endpoint in favor of two new endpoints that have the same functionality but provide consistent responses:

I'd like to use these endpoints in the library because the current methods make it awkward to test if a reference exists, but I was unsure what should happen to the existing methods.

  1. Modify the existing functions to use the new endpoints.

    • GetRef will behave the same but will return different concrete errors in the error cases (i.e. a standard ErrorResponse with a 404 code if the ref does not exist.)
    • GetRefs will mostly behave the same, but will return an empty list instead of an error if no refs match and will duplicate the functionality of ListRefs if an empty refs is used as an argument. The "error on empty" behavior could be retained if desired.
    • ListRefs will behave the same, but is technically redundant.
  2. Modify GetRef, leave GetRefs and ListRefs alone, and add a new ListMatchingRefs method. I think this will ultimately be confusing, there are 3 methods that do nearly the same thing, two of which are undocumented in the GitHub API.

  3. Modify GetRef, remove GetRefs and ListRefs, and add a new ListMatchingRefs method. This matches the API best, but is not backwards compatible and probably won't work for people still using GitHub Enterprise 2.18 and earlier.

What sounds best? I started implementing (1) but realized it might be better to cleanly break the API and add a new method, if breaking backwards compatibility is acceptable. I'm happy to implement whichever you are willing to support.


1The GitHub Enterprise 2.18 docs have the old endpoints, while the 2.19 docs have the new endpoints, putting the change between August and November, although github.com probably got it earlier.

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 29, 2020

Thank you for the research and the detailed analysis, @bluekeyes! It is greatly appreciated.

Yes, we prefer to match the official GitHub v3 API as closely as possible so that there is the least amount of confusion, even if this means bumping the major version of the repo due to the breaking API change (as can be seen, we are at v31 right now and already have another breaking change ready for v32 😄).

So I agree that Option # 3 sounds best, and welcome PRs. Please check out CONTRIBUTING.md which reminds you to run go generate ./... before pushing your PR.

Thanks again!

@bluekeyes
Copy link
Contributor Author

Thanks for the advice, @gmlewis! I implemented option 3 in #1513.

gmlewis pushed a commit that referenced this issue May 7, 2020
n1lesh pushed a commit to n1lesh/go-github that referenced this issue Oct 2, 2020
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 a pull request may close this issue.

2 participants