Skip to content

Conversation

@variableresistor
Copy link
Contributor

@variableresistor variableresistor commented Nov 30, 2022

Description

GitHub is introducing versioned API's. In order to get the expected behavior of an API, you need to invoke the API with its version in the request header. This change updates the core functions (Invoke-GHRestMethod and Invoke-GHRestMethodMultipleResult) to accept an ApiVersion as part of their param sets so that as the module continues to add new functionality, methods can request the appropriate API Version for their individual calls.

Features Added

Fixes #377

References

To infinity and beyond: enabling the future of GitHub’s REST API with API versioning
API Versions

Checklist

  • You actually ran the code that you just wrote, especially if you did just "one last quick change".
  • Comment-based help added/updated, including examples.
  • Static analysis is reporting back clean.
  • New/changed code adheres to our coding guidelines.
    - [ ] Formatters were created for any new types being added.
  • New/changed code continues to support the pipeline.
  • Changes to the manifest file follow the manifest guidance.
  • Unit tests were added/updated and are all passing. See testing guidelines. This includes making sure that all pipeline input variations have been covered.
  • Relevant usage examples have been added/updated in USAGE.md.
  • If desired, ensure your name is added to our Contributors list

@variableresistor
Copy link
Contributor Author

variableresistor commented Nov 30, 2022

@microsoft-github-policy-service agree company="City National Bank"

@variableresistor
Copy link
Contributor Author

@variableresistor please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

Contributor License Agreement

@microsoft-github-policy-service agree company="City National Bank"

@variableresistor variableresistor changed the title Added API versioning and Added API versioning and Project/Discussions support Nov 30, 2022
Copy link
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, @variableresistor! I also appreciate you calling attention to the fact that GitHub is introducing API versioning...I completely missed that announcement.

I love the idea of making the module API version aware, but I currently disagree with the methodology that you're using to introduce it. We're likely to adopt newer API versions on a per-API basis, one at a time. That means we wouldn't want a module-wide mechanism for changing the API Version, but rather the ability to change it on every REST call.

@ghost ghost added the needs-author-feedback label Dec 9, 2022
@ghost ghost removed the needs-author-feedback label Dec 13, 2022
@variableresistor variableresistor changed the title Added API versioning and Project/Discussions support Added API versioning support Dec 13, 2022
Copy link
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

I thought I had published this review earlier, but it still shows as pending.

Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>
@ghost ghost removed the needs-author-feedback label Dec 15, 2022
Copy link
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Super minor nits, and then should be good to merge.

Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>
@ghost ghost removed the needs-author-feedback label Dec 15, 2022
variableresistor and others added 2 commits December 15, 2022 11:15
Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>
Co-authored-by: Howard Wolosky <HowardWolosky@users.noreply.github.com>
@HowardWolosky
Copy link
Contributor

/azp run PowerShellForGitHub-CI

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks for the contribution and for noticing this pending change in the first place!

Tests are sufficiently passing (pretty much all of the failing tests are the Projects tests which we know will fail (due to #380) -- they're about to be disabled).

@HowardWolosky HowardWolosky merged commit 9388967 into microsoft:master Dec 16, 2022
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.

Add GitHub API versioning header

2 participants