Skip to content

Demo PR#25

Closed
RayStick wants to merge 10 commits intomasterfrom
demo-PR
Closed

Demo PR#25
RayStick wants to merge 10 commits intomasterfrom
demo-PR

Conversation

@RayStick
Copy link
Member

@RayStick RayStick commented Aug 26, 2025

Demo PR to show how the main reviewer checklist can work

  • Rachael opens a PR, so is assigned to this PR by default by the Auto Author Assign GH action (but this does not trigger the Main Reviewer Checklist which is good)
  • Rachael puts Stef as a reviewer, and as an assignee (to signify the main reviewer role)
  • Stef as a new assignee then triggers the GitHub action Main Reviewer Checklist which posts a comment

Unintended behaviour - but maybe we are okay with it?

  1. I also assigned Mary, and it directed the review comment to her as well
  2. I then deleted Stef as an assignee, but the comment remains

Both of these things are to be expected considering the yml workflow but do we want to update the workflow to try to stop these types of things? Or just assume that the PR author will only put one person in this slot.

Added section for main reviewer checklist.
@github-actions github-actions bot added the Documentation This issue or PR is about the documentation label Aug 26, 2025
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@RayStick RayStick assigned smoia and unassigned smoia Aug 26, 2025
@github-actions

This comment was marked as outdated.

@smoia
Copy link
Member

smoia commented Aug 26, 2025

This is awesome!
I think the point that stopped me was how to trigger the main reviewer checklist only once to the first person (self-)assigned to review, and then a more generic reviewer checklist to anyone else. That'd be a great solution IMHO, but IDK how to do it.

@RayStick
Copy link
Member Author

RayStick commented Aug 26, 2025

This is awesome!

Thanks =D

I think the point that stopped me was how to trigger the main reviewer checklist only once to the first person (self-)assigned to review, and then a more generic reviewer checklist to anyone else. That'd be a great solution IMHO, but IDK how to do it.

So just so I understand, the ideal scenario would be:

  1. Send a checklist (like the one above) to the main reviewer (main reviewer is the person added to the assignee slot and the reviewer slot)
  2. Send a checklist (much simpler than the one above) to all other reviewers (added to reviewer slot only)

If yes, I can give that a go

@RayStick RayStick marked this pull request as draft August 26, 2025 14:16
@smoia
Copy link
Member

smoia commented Aug 26, 2025

That could be good solution! Even better if the first person signing up as reviewer is also automatically assigned and prompted this message, vs any other reviewer that comes after.

@RayStick RayStick assigned RayStick and unassigned RayStick Aug 26, 2025
@RayStick RayStick assigned RayStick and unassigned RayStick Aug 26, 2025
@github-actions

This comment was marked as outdated.

@RayStick RayStick assigned smoia and unassigned smoia Aug 26, 2025
@github-actions

This comment was marked as outdated.

@RayStick RayStick removed the request for review from smoia August 26, 2025 16:55
@RayStick RayStick requested a review from smoia August 26, 2025 16:57
@github-actions
Copy link

Hello @!

You've been assigned to this PR, which means that you've been nominated as reviewer! You are a supporting reviewer, not the main reviewer - the main reviewer has more responsibilities than you (lucky them) but your review is still important.

If you have any doubt, check this section on reviewing

Checklist

  • one
  • two
  • three

Thank you!

@github-actions
Copy link

Hello @smoia!

You've been assigned to this PR, which means that you've been nominated Main Reviewer! Lucky you!

As a Main Reviewer, you are slightly more responsible for the quality of this PR than your fellow Reviewers. If you have any doubt, check this section on reviewing and this section on being the Main Reviewer of the documents.

Before merging this PR, please check that:

  • The PR passes all the CircleCI/Azure tests.
  • The content respects the Style Guide.
  • If the PR contains documentation, your local build is right.
  • The PR and the reviews are harmonious with the rest of the repository, especially that they don't introduce repetitions
  • The title of the PR is clear enough to describe a release content.
  • The labels are correct - and they will (or won't) trigger the correct auto release.
  • The PR received the amount of approvals necessaries to be merged.

If the PR contains code changes:

  • Content and changes are adequately documented in the docstrings.
  • User documentation is being updated accordingly - or a related issue has been opened.
  • The adequate tests have been added/updated - or a related issue has been opened if coverage doesn't drop below 90%.

After you merged this PR, please check that:

  • The Author(s) and Reviewers contributions have been updated in the README.
  • The updates (in code, tests and documentation) have appened correctly.
  • If they had to, the tag was created, the release was cut, and the pypi version got updated.

Thank you!

@RayStick RayStick requested a review from m-miedema August 26, 2025 16:59
@RayStick RayStick assigned smoia and RayStick and unassigned RayStick and smoia Aug 26, 2025
@RayStick
Copy link
Member Author

@smoia and @m-miedema feel free to unsubscribe from notifications whilst I am testing things, sorry!

@RayStick RayStick closed this Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation This issue or PR is about the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants