Optimize review process

[mtrezza]

New Feature / Enhancement Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.

Current Limitation

The GitHub review process is not transparent:

• For the developers it's unclear from whom to request a review.
• For the reviewer it is sometimes unclear whether a PR is ready for review if the PR is stale but no review was requested.
• Reviews are requested personally instead of from a (review) team and therefore burdening individual reviewers.

GitHub does not seem to offer a way to restrict review requests to teams or give teams preference.

Feature / Enhancement Description

A proposal for discussion:

1. When a PR is created, a bot will automatically apply the draft flag.
2. When the PR is ready for review, the contributor removes the draft flag.
3. A bot automatically requests a review from the review team of the repo.
4. Members of the review team (triage permission) make a review
5. When N reviews approved, the bot requests a merge from the maintenance team of the repo.
6. A member of the maintenance team (write permission) merges the PR.

Current restrictions:

• The write permission gives extensive permissions over the repo, so this should be restricted exclusively to a few people, if possible across repos. The skill to review a PR is not the same as the skill to master git commands and not mess up a repo.

Example Use Case

n/a

Alternatives / Workarounds

n/a

[parse-github-assistant]

Thanks for opening this issue!

• 🎉 We are excited about your ideas for improvement!

[dblythy]

I think it would be good to encourage contributors to submit their initial workings as "draft" reviews, so we can give them feedback on the technical approach prior to them perfecting their code. That way features that aren't going to be merged for certain reasons aren't required to meticulously refine test cases, syntax, etc. This would also help the collaborative approach of giving tips and pointers to new contributors for their draft PRs for where they should focus on, etc. This could allow newer developers to get involved without being expected to know the platform's ins and outs.

Also, not sure if I recall correctly, but I think it's currently not possible to request a review unless you are part of the Organization.

Of course, we would still expect issues to be opened to discuss issues and plan the approach, but the draft stage would be the first stage of the approach, where the commenters of the issue have the chance to vocalize issues and suggestions about the approach before the contributor spends considerable time on the issue.

[dblythy]

I really like the suggested approach here though Manuel and am looking forward to you creating (yet another) bot to help simplify our process for now and the future 😊

[dblythy]

Just thinking as well - should the bot auto-mark a review as a "draft" if there are merge conflicts, the PR is outdated with the default branch, or considerable tests are failing?

[mtrezza]

Also, not sure if I recall correctly, but I think it's currently not possible to request a review unless you are part of the Organization.

Yes, that is correct. The bot could do that though.

[mtrezza]

@sadortun I think you also previously suggested to set PRs to draft initially - what do you think about this process?

[sadortun]

A few ideas,

For the developers it's unclear from whom to request a review.

GH have an Auto assign reviewers feature. Create a Reviewer team, (no need for members to have write access) Make sure to check https://docs.github.com/en/organizations/organizing-members-into-teams/managing-code-review-assignment-for-your-team#configuring-code-review-assignment

If a PR went from Draft to Non-Draft after being reviewed, please don't set it back to draft. Draft should probably be used only to separate stuff thats WIP versus ready-ish)

You should have a look at @DefinitlyTyped repos. It have a great state management. 100% overkill for Parse, but a good inspiration https://github.com/DefinitelyTyped/dt-mergebot

https://github.com/DefinitelyTyped/DefinitelyTyped/pulls

Basically it use tags to tell the status

• Waiting for author
• Waiting for maintainer
• waiting for reviewers
• Test fails
• Waiting to resolve.discussions
• Invalid description ( your validation bot)
• Review approved
• Ready to merge

---

Also make sure to search for an existing bot for this. Plenty of repos already have something similar

[mtrezza]

Before anyone starts looking for a 3rd party GitHub Action or bot - we would be extending our custom Parse bot for any automation that we need in his regard.

[dblythy]

I think that's a good point @sadortun, maybe draft should be used for WIPs only and we should encourage PR contributors to request reviews from the respective reviewers where appropriate. We don't want features held up because PR reviewers aren't notified that features are ready, so perhaps we could encourage PR submitters to request reviews as they please.

[sadortun]

The bot could simply have a command for submited like :

• Press 🚀 if your PR is ready for review or ask for a new review, and the bot would ping the reviewers.

Once a reviewer submited comments, the submitter will have the name of the reviewer and could just ask directly.

If a reviewer thinks it's a WIP set the PR as Draft again