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

Send X-GitHub-Api-Version when calling [GitHub] v3 API #8669

Merged
merged 10 commits into from
Dec 31, 2022

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Nov 30, 2022

Closes #8665

This PR does three things:

  1. Sends the X-GitHub-Api-Version header when calling the GitHub v3 API
  2. Adds a scheduled GitHub workflow to:
    • Call https://api.github.com/versions
    • Find the latest version by date
    • Read in config/default.yml
    • If $date != public.services.github.restApiVersion:
      • sub in the new date + write config/default.yml
      • push to a branch
      • submit a PR update [github] api version
  3. Tangent: Fixes a long-standing bit of oddness: github-constellation.js has been ignoring the config.service.baseUri setting and reading straight from the env var. I guess most people either don't configure this or configure with env vars anyway, so it has gone unnoticed.

@chris48s chris48s added the service-badge Accepted and actionable changes, features, and bugs label Nov 30, 2022
@shields-ci
Copy link

shields-ci commented Nov 30, 2022

Warnings
⚠️ This PR modified the server but none of its tests.
That's okay so long as it's refactoring existing code.
⚠️

📚 Remember to ensure any changes to config.private in services/github/github-constellation.js are reflected in the server secrets documentation

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 08edb6c

@chris48s chris48s changed the title WIP send X-GitHub-Api-Version when calling GitHub v3 API WIP send X-GitHub-Api-Version when calling GitHub v3 API Nov 30, 2022
@chris48s chris48s changed the title WIP send X-GitHub-Api-Version when calling GitHub v3 API WIP send X-GitHub-Api-Version when calling [GitHub] v3 API Dec 4, 2022
@chris48s chris48s changed the title WIP send X-GitHub-Api-Version when calling [GitHub] v3 API Send X-GitHub-Api-Version when calling [GitHub] v3 API Dec 19, 2022
@chris48s chris48s marked this pull request as ready for review December 19, 2022 21:38
@calebcartwright
Copy link
Member

Fixes a long-standing bit of oddness: github-constellation.js has been ignoring the config.service.baseUri setting and reading straight from the env var.

Though a fix, imagine there's a non-zero chance of this impacting a self-hoster? May warrant a special call out in the next server relnotes

@chris48s
Copy link
Member Author

chris48s commented Dec 21, 2022

If you are currently setting this via GITHUB_URL then it will carry on working as before. services.github.baseUri is set from GITHUB_URL anyway

baseUri: 'GITHUB_URL'

so no change there.

I think the only case where this might cause a behaviour change would be if someone is hosting (using a source install, rather than docker), they edited services.github.baseUri in the yaml config (which would have no effect) and then they update to this release and suddenly their custom services.github.baseUri starts actually doing something.

Is that the case you were thinking about?

@calebcartwright
Copy link
Member

Is that the case you were thinking about?

Precisely. Someone who has a value in there that they forgot about which currently is a no-op but which would become operational once they pulled in this next version.

Obviously not saying we shouldn't do it, but it's the type of thing I'd put at/near the top of the release notes to call it out specially

@chris48s
Copy link
Member Author

Yep OK. I'll note this when I assemble the changelog for the January release. We need to call out the change to the GH workflows route there too.

@chris48s chris48s merged commit c3d08f7 into badges:master Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge Accepted and actionable changes, features, and bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version GitHub API requests
3 participants