[[PageOutline(2-3)]] = 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? == === Changesets === Low overhead. Only for ''post-commit review''. Review experimental repository clones / branches for ''pre-merge review''. === Files === Or parts of files. High overhead. Good for auditing? === Merge requests === Need to add the concept of a merge request to Trac first. But very useful and popular in modern DVCS systems. === Patches === Nice for projects that already rely on submitted patches for their development model. High overhead for projects that are used to direct commit access. === Screenshots === Optional bonus feature? == Configurable Workflow == Code reviews should support configurable workflows (like tickets). === Relevant states === * `new` * A new code review has been created * `closed` * The code review has concluded. (Like for tickets, various resolutions may clarify what this means.) * `assigned`, `reviewing`, `reviewed`? * 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.) === Relevant resolutions === * `accepted` / `rejected` * For ''acceptance code review'' that results in a vote on inclusion. * `completed` * For ''educational code review'' to increase knowledge transfer. * `invalid` (`ignore`, `irrelevant`, `retracted`, `superseded`, `outdated`, `noneed`, `nntr` (''No Need To Review''))) * For various cases where a code review is aborted. === Assigning reviewers === 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. === Voting === 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 `accepted` or `rejected` states. === Veto === Some / all users may have a ''veto'' permission to overrule other votes instantly? == Commenting == 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. == VCS integration == * 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. * `Unreviewed` link to the form to start a new review. * `Reviewed` links to the review. * What if only an old revision was reviewed? * For directories show the percentage of reviewed files? == Ticket integration == * Should a code review simply be a ticket of ''ticket type'' `codereview`? * 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: * Notification * People need to be notified when they are assigned as a reviewer or the status of their code review changes. * Timeline * The timeline should show the important review events. * Search * Reviews and review comments should be searchable. * Wiki * 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.