btw maybe the github review feature could be leveraged? eg. https://docs.github.com/en/github/managing-your-work-on-github/filtering-pull-requests-by-review-status (i'm not sure if it meets the needs but maybe it could)

@mattab it doesn't really help unfortunately

👍 could help. would make things clearer for me at least (though it's not a big time sink for me atm to just look through all the PRs in the current milestone and see what can be merged/what needs reviews)

Sounds fine I guess.
we could maybe also use the review states more actively. Once the required changes have been done by the author it might maybe also be helpful to dismiss reviews that should have been solved with new commits. If multiple peoples are doing reivew we might otherwise end up with having multiple states from different reviewes within one PR.

@diosmosis comment about the milestone is very helpful too:

for me atm to just look through all the PRs in the current milestone and see what can be merged/what needs reviews

It means if I work on something which has a milestone in the future but finished earlier, I should set the milestone to the very next one?

I like the idea to set/unset labels. As the team grows, more and more active PRs will have, this could really help quickly go through.

@flamisz no, but it would help to set the milestone to the same one as the issue. What goes into the next milestone has to do with the value a change might bring & the potential cost of introducing it into live systems & making it stable, rather than what is coded and ready to go at any given moment. And @mattab & @tsteur are the ones in charge w/ prioritization at this level, so we wouldn't change that unless we knew it was a valuable enough or a small enough change.

trying to summarise the thoughts so far from previous comments:

• The PR author adds "Needs Review" label and assigns the PR to a milestone (usually the same milestone as the referenced issue) once the PR is ready for a review
• The reviewer selects "Approve / Reject changes / Comment" when finishing a review so it's clear if a
  PR is accepted or more changes are needed
• The PR author marks an individual review conversion/comment as resolved when the feedback was applied or notices/acknowledged (not always do need changes to be made for each comment as sometimes they are suggestions etc)
  • If further feedback is needed for the conversation/comment then PR author will ask the reviewer to confirm the conversation is resolved
• The reviewer removes the label "needs review" when changes are required. No new label is added for now for simplicity (but could also add on)
• The PR author adds the label "needs review" again once it's ready for another review
• When a PR is approved and no further review required then we aim to directly merge the PR if the tests pass
  • If a PR is approved overall but tests are failing then we might need another round of review (meaning reject the PR and remove needs review label)
  • unless it is only needed to update the expected test results then maybe the PR author could later fix the build and merge directly without yet another review (or maybe only if the PR reviewer mentions that only test )... could clarify this further in a team meeting some day

Does this overall sound in the right direction? Any thoughts / objections? Also of course we can change/improve things any time .

Overall the goal again be to easily be able to see which issues currently need a review etc and trying to get approved PRs merged as quick as possible (for example so less potential merge conflicts happen etc)

I like it, it's a very clean and simple workflow.

Great, I've put this into the docs and I'll close this for now and we can change it any time. Any feedback any time be greatly appreciated. :)