User:ThomasDinges/ImprovingPatchReview

Improving patch review

Patch review is an essential part of the development process. This report lists current issues and discusses possible improvements, to make it easier for new contributors and more efficient for reviewers.

Transparency: Some ideas mentioned in this report, were first mentioned by other Blender developers and summarized by me.

Issues

A quick summary of current issues and difficulties.

• Long response times / no response at all
• Not 100% clear who to add as reviewer, especially if code touches many areas. Who is ultimately responsible and can decide?
• Patches abused as design tasks: Even though there is no decision on (basic) design yet, code is already reviewed and changes are made.
• Not enough people / time to review all patches
• Insufficient code quality / lack of experience

Ideas to improve the process

Strengthen module teams

In order to have a working review process in the first place, we need to ensure modules are organized well and have the resources to review code.

• Make sure the module itself has a clear vision (roadmaps, targets, list of approved features...).
• Have requirements and specifications for the module, and designs for new development for the 3.x series.
• Strengthen module owners to commit (or approve commits from members) without patch review for smaller tasks.
• Allow mistakes to happen, but ensure modules act quickly and learn from these.

On the other hand, if this is not possible (e.g. when there are not enough maintainers) and the module is in maintenance mode, communicate that clearly. Limit any contribution to non-disputed ones and otherwise put on hold.

Ensuring code / design quality

Keeping code quality high and only accept features with approved design is important. It reduces extra work for developers and hopefully avoids situations where a patch stays in limbo either because the design is not accepted or the contributor gives up frustrated. For that we have to communicate better what we expect.

Some ideas, to improve code quality were already mentioned in my Onboarding & Modules report. Other ideas:

Prioritize some patches

We could put a focus on patches, which link to a design task or a bug report. Patches that have no approved design or don't fix a bug, would then be considered lower priority, and we can more easily close them if quality is low. Exceptions could be approved module work, code cleanup, refactors...

In simple ways, this would be an extra highway lane, which hopefully motivates people to think about design and code quality, before submitting a patch. At the same time it will reduce frustration if a patch is rejected, because the expectations are clearer for contributors.

Improve focus and response times

Maintain a list of actively worked on patches

Having a list of patches to focus on, can improve response times, increase responsibility and also makes it clear for the community, what is actively being worked on. Also sometimes patches touch many areas / modules and require review and feedback from all of them. These cases can be improved as well. Idea:

• We create a new empty list for these patches, e.g. on the wiki.
• Every module can put one open patch which they find important onto this list.
• Other modules involved will help reviewing these patches.
• Once the patch is approved and committed, the module can put a new patch onto the list.

This way, every module focuses on one important patch. Involved modules are encouraged to check on the patch too, as they want other modules to look at theirs in return.