Introduce PR review policy

[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

Active contributors are the heart beat of any open source project. As such, their contributions and investments of time deserve timely feedback.

Currently there is no guideline regarding PR reviews that determines the time frame in which PRs are reviewed and feedback is given. This leads to:

• Additional work for PR author to resolve merge conflicts due to regular PRs, e.g. Snyk.
• Contributors may become demotivated by not getting a review within reasonable amount of time; a review takes maybe 5-15 minutes while a PR often takes hours of work.
• Improvements of product, docs and frameworks are unnecessarily delayed.
• PRs are more likely to be abandoned due to low feedback frequency.

It is also not clear who to tag for review, as there are multiple members in each repo group, some of which do not seem to be active anymore.

Feature / Enhancement Description

• Introduce a review policy that aims to provide feedback on PRs within a defined amount of time.
• Actively recruit and onboard new repo team members to increase review capacity.
• Update team members of repo group, e.g. @ parse-community/server and set a guideline to tag the server group instead of individual members, so that review load is better distributed among reviewers.

Example Use Case

(none)

Alternatives / Workarounds

(none)

3rd Party References

(none)

[dblythy]

I agree, although I think the community understands that the core maintainers have their own projects and schedules to uphold, and that it takes a notable period of time for the core team to review every PR for quality. I imagine this can sometimes be tedious for rather large PRs, especially when you guys have been doing this for a few years now. Not to mention the amount of time you guys spend triaging issues, in the forums, etc.

Personally, I would happy to see the core team receive a small financial incentive from the OpenCollective for time spend PR reviewing or something. As a backer and contributor myself, I know you, Diamond and others spend considerable time going through my PRs, and I would be happy to see some of my donation reward you for your efforts in ensuring Parse Server continues to grow for the better. I know we've had this conversation first around "community first", but I think the worst thing for the community would be for you PR reviewers to become disinterested, and the quality of new PRs becoming worse.

[TomWFox]

• Introduce a review policy that aims to provide feedback on PRs within a defined amount of time.

What sort of timeframe do you have in mind?

I don't think it's feasible unless the time guide was really long (1-3 weeks perhaps). In an ideal world where we had 2 or 3 maintainers paid through OC (or the companies they work for) working a couple days a week then a short timeframe could work.

As it is, if we had a short timeframe (i.e. 3-7 days) I think that sets the team up for failure and could be demotivating. Reviewers should be reviewing when it suits them, they are in the mood etc. If reviewers feel in a rush or demotivated the quality of review could also go down.

I disagree with the suggestion of a review taking 5-15 mins, obviously it varies wildly on the type of PR and volume of changes but if I was guessing I would say I spend an average of 30 mins on a first pass.

• Actively recruit and onboard new repo team members to increase review capacity.

Always in favour of this, the main issue is with the limited supply of people willing and able. I'm not sure of any changes we could make currently to meaningfully improve this but absolutely open to ideas - perhaps in a separate discussion?

*Update team members of repo group, e.g. @ parse-community/server and set a guideline to tag the server group instead of individual members, so that review load is better distributed among reviewers.

I always add people to teams (that's how the perms are managed), are there any specific changes you are thinking of or just speaking generally?

I very much agree with tagging the team, it also reduces any unwritten pressure/expectation that can come when an individual tagged for review. There would still cases when tagging an individual is appropriate.

[mtrezza]

Ideally I would like us to go from the current undefined status quo to a loosely defined time frame.

I don't mean time until merging, but just until a feedback or review, even if it is just partly. I personally often review in chunks, high level down. That lets the author work on changes and I can review PRs in parallel and overall increase feedback frequency. Letting someone wait for 3 weeks for an initial feedback seems too long to me, given the emotional component of receiving feedback for effort.

There are some critical tasks that could benefit from defined time frames, such as vulnerability fixing or bug fixing and, I would say also PR feedback / review.

But I agree with your points, it needs to be a balance of enough time to review and timely feedback.

I just noticed that PRs sometimes stay without initial or intermediate feedback for extended periods of time and wonder if we can find a way to improve that.

I though the outcome of this would be adding a bot, but a high level discussion may be better suited in the forum.