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 updated GitHub Pages Update API #2406

Closed
gesellix opened this issue Jul 6, 2022 · 1 comment · Fixed by #2407
Closed

Support updated GitHub Pages Update API #2406

gesellix opened this issue Jul 6, 2022 · 1 comment · Fixed by #2407
Assignees
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).

Comments

@gesellix
Copy link
Contributor

gesellix commented Jul 6, 2022

In former versions of the GitHub API the pages update request used a source field of type string, with the branch name and a path delimited by a space. According to the current GitHub documentation and experiments with curl, the update api apparently now also supports the source parameter to be a struct like {Branch: string, Path: string}.

Looking at the required changes I think that this might break existing consumers of go-github using the PagesUpdate request - correct me if I'm wrong. I'm not sure if you prefer to have some kind of migration path supporting both versions or if a breaking change would be tolerable (maybe communicated with a major version bump). If you have any preference, please leave a note or a code pointer with an example how you'd like me to handle this.

See integrations/terraform-provider-github#1198 for the motivation, and #2371 where the same requirement came up already. The planned change would also make the create and update pages code more consistent.

I've already started on a patch and I'm going to submit the pull request in the next days, so we might have a discussion on the actual change.


To show the desired effect, here are some examples using curl:

curl -X PUT https://api.github.com/repos/gesellix/pages-test/pages \
  -H 'Accept: application/vnd.github.v3+json' \
  -H 'Content-Type: application/json' \
  -H 'User-Agent: go-github' \
  -d '{"source":"main /docs"}' \
  -w "\n%{http_code}\n"
{"message":"Invalid request.\n\nInvalid property /source: `main /docs` is not a possible value. Must be one of the following: gh-pages, master, master /docs.","documentation_url":"https://docs.github.com/rest/reference/repos#update-information-about-a-github-pages-site"}
422
curl -X PUT https://api.github.com/repos/gesellix/pages-test/pages \
  -H 'Accept: application/vnd.github.v3+json' \
  -H 'Content-Type: application/json' \
  -H 'User-Agent: go-github' \
  -d '{"source":{"branch":"main","path":"/docs"}}' \
  -w "\n%{http_code}\n"

204
curl -X PUT https://api.github.com/repos/gesellix/pages-test/pages \
  -H 'Accept: application/vnd.github.v3+json'\
  -H 'Content-Type: application/json'\
  -H 'User-Agent: go-github'\
  -d '{"source":{"branch":"master","path":"/docs"}}'\
  -w "\n%{http_code}\n"

204
curl -X PUT https://api.github.com/repos/gesellix/pages-test/pages \
  -H 'Accept: application/vnd.github.v3+json'\
  -H 'Content-Type: application/json'\
  -H 'User-Agent: go-github'\
  -d '{"source":{"branch":"gh-pages","path":"/docs"}}'\
  -w "\n%{http_code}\n"

204
curl -X PUT https://api.github.com/repos/gesellix/pages-test/pages \
  -H 'Accept: application/vnd.github.v3+json' \
  -H 'Content-Type: application/json'\
  -H 'User-Agent: go-github'\
  -d '{"source":{"branch":"gh-pages","path":"/wiki"}}'\
  -w "\n%{http_code}\n"
{"message":"Invalid request.\n\nInvalid property /source: `/wiki` is not a possible value. Must be one of the following: /, /docs.","documentation_url":"https://docs.github.com/rest/reference/repos#update-information-about-a-github-pages-site"}
422
curl -X PUT https://api.github.com/repos/gesellix/pages-test/pages \
  -H 'Accept: application/vnd.github.v3+json'\
  -H 'Content-Type: application/json'\
  -H 'User-Agent: go-github'\
  -d '{"source":{"branch":"gh-pages","path":"/"}}'\
  -w "\n%{http_code}\n"

204
@gmlewis
Copy link
Collaborator

gmlewis commented Jul 7, 2022

I'm not sure if you prefer to have some kind of migration path supporting both versions or if a breaking change would be tolerable (maybe communicated with a major version bump).

Yes, breaking changes are made to this API on a regular basis due to the moving target we are attempting to hit. 😄

So a PR is definitely welcome, and we will mark it as a breaking API change, and then remember to bump the major version of this repo.

Thank you, @gesellix !

@gmlewis gmlewis added the Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants