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

Update Protected Branches API preview #617

Closed
gmlewis opened this issue Apr 13, 2017 · 13 comments
Closed

Update Protected Branches API preview #617

gmlewis opened this issue Apr 13, 2017 · 13 comments

Comments

@gmlewis
Copy link
Collaborator

gmlewis commented Apr 13, 2017

GitHub Announcement:
https://developer.github.com/changes/2017-04-11-protected-branch-dismissal-restrictions/

Note that the provided link in the announcement is somewhat misleading in that this change affects potentially more than just this one API endpoint on this page.

I think the point is to ensure that dismissal_restrictions is handled properly.

@jkosecki
Copy link
Contributor

I can work on this. @gmlewis, please assign it to me

@gmlewis
Copy link
Collaborator Author

gmlewis commented Apr 27, 2017

Please also check out the latest GitHub Developer announcement about this API:
https://developer.github.com/changes/2017-04-20-protected-branches-dismiss-stale-review/
If you can incorporate these changes as well, it seems reasonable to me to do so within the same PR since it is the same preview API.

If not, though, that is fine too and we can open up a new issue for it.

@jkosecki
Copy link
Contributor

jkosecki commented May 1, 2017

@gmlewis No problem, I'll bring this changes as well, no need to create another issue

@gmlewis
Copy link
Collaborator Author

gmlewis commented May 4, 2017

There is yet another GitHub Developer announcement about breaking changes to this API:
https://developer.github.com/changes/2017-05-02-adoption-of-admin-enforced/

jkosecki added a commit to jkosecki/go-github that referenced this issue May 10, 2017
jkosecki added a commit to jkosecki/go-github that referenced this issue May 10, 2017
jkosecki added a commit to jkosecki/go-github that referenced this issue May 10, 2017
@gmlewis
Copy link
Collaborator Author

gmlewis commented Jun 22, 2017

There is another announcement for this API:
https://developer.github.com/changes/2017-06-16-loki-preview-ending-soon/

@gmlewis
Copy link
Collaborator Author

gmlewis commented Jun 28, 2017

I believe that this issue is still open, so I didn't close it with #637.
Please correct me if I'm wrong as I haven't done a thorough investigation at this point.

@elliott-beach
Copy link
Contributor

elliott-beach commented Aug 22, 2017

The Jun 22 announcement did not introduce new changes, but it means we can remove the special accept header for the API preview on Sept 1st.

To elaborate on the gory details of the announcement, the functions related to protected branches such as updateBranchProtection already work in the way described in the announcement. The update endpoint uses the PUT method, and we have a requiredPullRequestReview property of the struct which the client can populate. https://github.com/google/go-github/blob/fe4b6036cb400908b8903d3df9444b9599a60d57/github/repos.go#L773-774.

I can take of care of removing the accept header, although it is a very simple change.

elliott-beach added a commit to elliott-beach/go-github that referenced this issue Aug 22, 2017
elliott-beach added a commit to elliott-beach/go-github that referenced this issue Aug 22, 2017
@gmlewis
Copy link
Collaborator Author

gmlewis commented Aug 22, 2017

While here, @e-beach, can you determine if removing the API preview accept header would affect Enterprise API clients? I seem to recall that if you are not a GitHub Enterprise customer, that you don't have access to the Enterprise API docs, but this was a long time ago and I could be mistaken. We just had a discussion about this in #638.

@elliott-beach
Copy link
Contributor

elliott-beach commented Aug 23, 2017

@gmlewis I couldn't tell. I suspect we must wait and see until the documentation is updated, either in the current release, 2.10, or in a new release.

The enterprise docs are available at https://developer.github.com/enterprise/2.10/v3/, and there's contextual information and a list of releases at https://developer.github.com/v3/enterprise/.

It seems correct to not use those above commits of mine until we know for certain that the change is supported on Enterprise.

@gmlewis
Copy link
Collaborator Author

gmlewis commented Sep 7, 2017

Latest announcement from GitHub:
https://developer.github.com/changes/2017-09-06-protected-branches-preview-end/

It is still not clear to me if this equally applies to Enterprise customers.
Maybe an Enterprise customer could comment.

@elliott-beach
Copy link
Contributor

Actually, documentation on this is available here for the most recent version of GitHub Enterprise: https://developer.github.com/enterprise/2.10/v3/repos/branches/#response-1. It still lists the API as in preview mode.

My understanding is that, because GitHub Enterprise is locally hosted by the enterprise, enterprise customers have to upgrade their infrastructure to receive new updates - it doesn't happen automatically. GitHub has to release a new version of the Enterprise API, and customers have to upgrade to the new version. So it would be best to maintain the preview header for some time yet.

@dmitshur
Copy link
Member

dmitshur commented Sep 9, 2017

My understanding is that, because GitHub Enterprise is locally hosted by the enterprise, enterprise customers have to upgrade their infrastructure to receive new updates - it doesn't happen automatically. GitHub has to release a new version of the Enterprise API, and customers have to upgrade to the new version. So it would be best to maintain the preview header for some time yet.

Right. But we still need to have some goal that we can target. I think it should be the latest Enterprise version that's available. When it no longer requires the preview header, that's when we can remove it.

The idea is that those who are using latest enterprise version can use latest version of go-github. Others should use an older version via vendoring and update when they update their enterprise version.

If we don't have a clear goal of when we can remove preview headers, it would become very hard to know when to remove them. They would accumulate and begin to cause issues and increased maintenance and development cost.

@gmlewis
Copy link
Collaborator Author

gmlewis commented Aug 6, 2021

Closing issue as obsolete.

@gmlewis gmlewis closed this as completed Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@dmitshur @jkosecki @gmlewis @elliott-beach and others