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

Add List Installation Requests API #2947

Merged

Conversation

abhijit-hota
Copy link
Contributor

@abhijit-hota abhijit-hota commented Oct 3, 2023

The PR adds support for listing installation requests for the authenticated app.

  • Feature addition
  • Added Tests
  • Run the essential scripts for linting, generation etc.

@google-cla
Copy link

google-cla bot commented Oct 3, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@abhijit-hota abhijit-hota marked this pull request as ready for review October 3, 2023 05:11
@abhijit-hota
Copy link
Contributor Author

Signed CLA and rescanned.

@abhijit-hota abhijit-hota changed the title Add support for installation requests api Add support for installation requests API Oct 3, 2023
@abhijit-hota abhijit-hota changed the title Add support for installation requests API Add List Installation Requests API Oct 3, 2023
@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Merging #2947 (3bc5b0b) into master (8cd452b) will decrease coverage by 0.03%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #2947      +/-   ##
==========================================
- Coverage   98.19%   98.17%   -0.03%     
==========================================
  Files         145      145              
  Lines       12720    12735      +15     
==========================================
+ Hits        12490    12502      +12     
- Misses        156      158       +2     
- Partials       74       75       +1     
Files Coverage Δ
github/apps.go 93.52% <80.00%> (-1.64%) ⬇️

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @abhijit-hota !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Oct 3, 2023
@abhijit-hota
Copy link
Contributor Author

Thanks, @gmlewis. I see that the coverage has decreased somehow. Can I do anything about it?

@gmlewis
Copy link
Collaborator

gmlewis commented Oct 3, 2023

Thanks, @gmlewis. I see that the coverage has decreased somehow. Can I do anything about it?

I don't think so, because it is harder to test the endpoints that take no parameters in the URL itself... as we can't corrupt it as easily... so I'm not worried about the dropping code coverage in this case.

Thank you for checking, though, @abhijit-hota ! It is appreciated. ❤️

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Oct 3, 2023
@gmlewis
Copy link
Collaborator

gmlewis commented Oct 3, 2023

Thank you, @WillAbides !
Merging.

@gmlewis gmlewis merged commit 7d26b99 into google:master Oct 3, 2023
5 of 7 checks passed
@abhijit-hota
Copy link
Contributor Author

@gmlewis Thank you for the prompt review and merge!

Is there any tentative release date for this?

@gmlewis
Copy link
Collaborator

gmlewis commented Oct 4, 2023

@gmlewis Thank you for the prompt review and merge!

Is there any tentative release date for this?

It might be nice to get #2937, #2948, and #2949 merged before cutting the new release.
If you feel like moving the process along by contributing to code reviews, that would be greatly appreciated.

@abhijit-hota
Copy link
Contributor Author

Hi @gmlewis, I see that the above PRs have been merged!

We're currently using the master branch and would prefer to shift to a pinned release.

Thanks!

@gmlewis
Copy link
Collaborator

gmlewis commented Oct 12, 2023

@abhijit-hota - this has now been included in the latest release:
https://github.com/google/go-github/releases/tag/v56.0.0

@abhijit-hota
Copy link
Contributor Author

Thanks for the quick response again, @gmlewis!

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.

None yet

3 participants