|Version 1 (modified by 8 years ago) ( diff ),|
Code Review Wishlist
This page contains some wishful thinking about a hypothetical ideal code review system built into Trac (or a plugin).
What to review?
Low overhead. Only for post-commit review. Review experimental repository clones / branches for pre-merge review.
Or parts of files. High overhead. Good for auditing?
Need to add the concept of a merge request to Trac first. But very useful and popular in modern DVCS systems.
Nice for projects that already rely on submitted patches for their development model. High overhead for projects that are used to direct commit access.
Optional bonus feature?
Code reviews should support configurable workflows (like tickets).
- A new code review has been created
- The code review has concluded. (Like for tickets, various resolutions may clarify what this means.)
- Shows when a reviewer has been assigned, has started or finished reviewing the code.
- This may be complicated by the need for multiple reviewers in one review.
- Avoid over-complicating the process? (It's configurable. Some may need a more advanced workflow, some want to keep a low overhead.)
- For acceptance code review that results in a vote on inclusion.
- For educational code review to increase knowledge transfer.
nntr(No Need To Review)))
- For various cases where a code review is aborted.
A new code review should be assignable to specific reviewers (Trac users) or to a group of reviewers (permission groups?). Depending on the workflow, one, some or all of them should have to perform some action on the review.
In some workflows, the individual reviewers are required / allowed to vote on a code review. Voting could be a state transition. A special transition operation could tally and check the votes against a configured threshold. Depending on the outcome it remain in the same state (and wait for further votes) or resolve the review to the
Some / all users may have a veto permission to overrule other votes instantly?
Reviewers and submitter need to discuss the reviewed code. Referring to specific section or lines of code should be easy, by attaching and showing comments and replies inline / side-by-side with the code.
- Should a new changeset automatically create a new review?
- Should a special changeset commit message keyword trigger a new review? (and assign reviewers?)
- similar to / extending CommitTicketUpdater?
- Should the existing changeset / diff / repository view pages be extended or modified?
- If files are reviewable, the browser could show the Review status of a file.
Unreviewedlink to the form to start a new review.
Reviewedlinks to the review.
- What if only an old revision was reviewed?
- For directories show the percentage of reviewed files?
- Should a code review simply be a ticket of ticket type
- We get some discussion, notification and workflow support for free.
- For reviewing patches / merge requests submitted as proposed fixes for the ticket's issue this seems ideal.
- Should a reviewer open new tickets for each complaint?
- Should a rejected post-commit code review result in one new ticket?
New navigation tab
If not all code review features are integrated into existing pages, a new navigation tab is needed where code reviews can be created and viewed etc.
One should see (be able to query?) ones submitted and assigned reviews and their most important properties. (Did I or someone else comment or reply?)
Should reviews be prepared and performed in a private staging area and only published when ready?
Other Trac integration
The code review system should be "natively" integrated in Trac in the sense that it works together with the appropriate core systems:
- People need to be notified when they are assigned as a reviewer or the status of their code review changes.
- The timeline should show the important review events.
- Reviews and review comments should be searchable.
- Are there any appropriate special macros, link syntax, etc.?
- Preferences, Admin, Permissions
- Configuration of code review permissions, admin settings and user preferences should be provided in the standard Trac way.