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 support DeploymentReview Event, ReviewCustomDeploymentProtectionRule API, GetPendingDeployments API #3254

Merged
merged 6 commits into from
Sep 26, 2024

Conversation

air-hand
Copy link
Contributor

@air-hand air-hand commented Aug 29, 2024

@air-hand air-hand changed the title WIP issue-3252 Add support DeploymentReview Event, ReviewCustomDeploymentProtectionRule API, GetPendingDeployments API Sep 3, 2024
@air-hand air-hand marked this pull request as ready for review September 3, 2024 13:05
Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.95%. Comparing base (2b8c7fa) to head (2328ae7).
Report is 105 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3254      +/-   ##
==========================================
- Coverage   97.72%   92.95%   -4.77%     
==========================================
  Files         153      171      +18     
  Lines       13390    11680    -1710     
==========================================
- Hits        13085    10857    -2228     
- Misses        215      729     +514     
- Partials       90       94       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Sep 3, 2024
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, @air-hand, this is looking great!
Just a couple minor nits to address, please, then we will be ready for a second LGTM+Approval from any other contributor to this repo before merging.

github/actions_workflow_runs.go Show resolved Hide resolved
github/actions_workflow_runs.go Outdated Show resolved Hide resolved
@air-hand
Copy link
Contributor Author

air-hand commented Sep 3, 2024

Thank you, @air-hand, this is looking great! Just a couple minor nits to address, please, then we will be ready for a second LGTM+Approval from any other contributor to this repo before merging.

@gmlewis
Thanks for the review!
I have applied your suggestions.

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, @air-hand !
LGTM.

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

@air-hand
Copy link
Contributor Author

Hello, @gmlewis.

I am waiting for a second LGTM+ approval from other contributors to this repository before merging it.

I know this repository is supported by volunteers, but I want to release this.
If possible, could you please tell me how to find a second reviewer as PR author? 🙏

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 25, 2024

If possible, could you please tell me how to find a second reviewer as PR author?

Do you have a coworker you could ask to perform the code review?

@air-hand
Copy link
Contributor Author

Do you have a colleague you can ask to review code?

Probably not.
I think that PR reviewers for this repository must be contributors, cause I can't assign myself as a reviewer other PR just now.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 25, 2024

I think that PR reviewers for this repository must be contributors, cause I can't assign myself as a reviewer other PR just now.

We welcome new contributors to this repo all the time. All new Go developers are welcome to contribute with PRs or code reviews. See CONTRIBUTING.md for more information.

@air-hand
Copy link
Contributor Author

Oh, I mean that a user who has never committed to the default branch (i.e., a user who is not listed as a "contributor") may not be able to assign itself as a reviewer on github because I have tried other PR but I couldn't.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 25, 2024

I've asked @srgustafson8 if a code review might be possible since @srgustafson8 is actively working on other PRs right now...

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 25, 2024

Oh, I mean that a user who has never committed to the default branch (i.e., a user who is not listed as a "contributor") may not be able to assign itself as a reviewer on github because I have tried other PR but I couldn't.

Just FYI - it is almost never necessary that a reviewer be assigned in this repo on any particular PR.
We welcome all code reviews and rarely ever assign specific reviewers.

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 25, 2024

In fact, @air-hand - please feel free to perform code reviews yourself on any open PRs that have the "Needs Review" label... all contributions are greatly appreciated!

@air-hand
Copy link
Contributor Author

Just FYI - it is almost never necessary that a reviewer be assigned in this repo on any particular PR.

Thanks @gmlewis, I finally understand. (I had always assign reviewer myself on PR before code review...)

@gmlewis
Copy link
Collaborator

gmlewis commented Sep 25, 2024

Thanks @gmlewis, I finally understand. (I had always assign reviewer myself on PR before code review...)

Yes, that is totally reasonable and quite common especially in a corporate environment.
No worries at all. Thanks again for all your help to make this repo better! It is greatly appreciated.

@srgustafson8
Copy link
Contributor

@air-hand @gmlewis will try and get to this one today :)

Copy link
Contributor

@srgustafson8 srgustafson8 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 to me

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 26, 2024
@gmlewis
Copy link
Collaborator

gmlewis commented Sep 26, 2024

Thank you, @srgustafson8 !
Merging.

@gmlewis gmlewis merged commit 3d410c2 into google:master Sep 26, 2024
6 of 7 checks passed
gmlewis pushed a commit that referenced this pull request Oct 1, 2024
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.

Feature request: support Deployment Review Event, Review Custom Deployment Protection Rule API
3 participants