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.

Sign up for GitHub

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

Jump to bottom

Also fail DO-NOT-MERGE when "awaiting changes" or "awaiting change review" present on PR #103807

Merged
merged 1 commit into from
Apr 24, 2023
Merged

Also fail DO-NOT-MERGE when "awaiting changes" or "awaiting change review" present on PR #103807

merged 1 commit into from
Apr 24, 2023

Conversation

ambv
Copy link
Contributor

@ambv ambv commented Apr 24, 2023

"awaiting changes" means somebody put a review that requested changes.

"awaiting change review" means that the PR author published changes after a red review and then requested a re-review.

Mariatta
Mariatta previously requested changes Apr 24, 2023
View reviewed changes
Copy link
Member

@Mariatta Mariatta left a comment

Choose a reason for hiding this comment

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

Re-review this

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@hugovk
Copy link
Member

hugovk commented Apr 24, 2023

If we merge this, we need to add this new job name as a required check:

image

And remove the old one.

We'll have some PRs out of sync (unless we keep the old name).

@ambv
Copy link
Contributor Author

ambv commented Apr 24, 2023

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@Mariatta: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from Mariatta April 24, 2023 23:12
@ambv
Copy link
Contributor Author

ambv commented Apr 24, 2023

(I didn't actually make the changes, I wanted to see if it still jumps to the red check, which it did.)

@ambv
Copy link
Contributor Author

ambv commented Apr 24, 2023

I believe we sadly have to change the name of the check because it is otherwise too misleading for the case of "awaiting changes".

@ambv ambv added the needs backport to 3.11 only security fixes label Apr 24, 2023
@ambv ambv dismissed Mariatta’s stale review April 24, 2023 23:17

Is the check green after there's no red review?

@ambv
Copy link
Contributor Author

ambv commented Apr 24, 2023

It does!

@ambv ambv merged commit b51da99 into python:main Apr 24, 2023
@miss-islington
Copy link
Contributor

Thanks @ambv for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@ambv ambv deleted the auto-merge-unresolved-review branch April 24, 2023 23:50
@ambv ambv restored the auto-merge-unresolved-review branch April 24, 2023 23:58
ambv added a commit to ambv/cpython that referenced this pull request Apr 24, 2023
@ambv
[3.11] Also fail DO-NOT-MERGE when "awaiting changes" or "awaiting ch…
1e54939 
…ange review" present on PR (pythonGH-103807)

"awaiting changes" means somebody put a review that requested changes.

"awaiting change review" means that the PR author published changes
after a red review and then requested a re-review.
(cherry picked from commit b51da99)

Co-authored-by: Łukasz Langa <lukasz@langa.pl>
@bedevere-bot
Copy link

GH-103814 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Apr 24, 2023
@ambv ambv deleted the auto-merge-unresolved-review branch April 25, 2023 00:36
@ambv ambv restored the auto-merge-unresolved-review branch April 25, 2023 00:41
@ambv ambv deleted the auto-merge-unresolved-review branch April 25, 2023 00:42
ambv added a commit that referenced this pull request Apr 25, 2023
@ambv
[3.11] Also fail DO-NOT-MERGE when "awaiting changes" or "awaiting ch…
99db23d 
…ange review" present on PR (GH-103807) (#103814)

"awaiting changes" means somebody put a review that requested changes.

"awaiting change review" means that the PR author published changes
after a red review and then requested a re-review.

(cherry picked from commit b51da99)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

@hugovk hugovk Awaiting requested review from hugovk hugovk is a code owner

@Mariatta Mariatta Awaiting requested review from Mariatta

Assignees
No one assigned
Labels
skip issue skip news
Projects
CPython labels
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ambv @bedevere-bot @hugovk @miss-islington @Mariatta