My team frequenty uses a feature branch workflow, where we develop features incrementally on a separate branch and then merge them into master. Individual changes are reviewed as they are committed to the feature branch. The problem happens when the changes are merged into master, generating a merge commit, which is then automatically audited, genenrating a (usually large) audit containing already-reviewed changes. Some more details below:
Given a package with automatic audit enabled it is fairly easy to get in a situation users are forced to re-audit changes during true merges.
Consider the following situation, involving a master branch and a feature branch. Assume all commits mentioned affect some package which has auto-audit enabled.
feature: A - B - C \ master: ... - D - E
Now, if commits A, B, C, all have audits, and commit E is a simple merge commit that merges those changes into master, then triggering an audit on E basically presents cumulative changes from A, B, C combined for audit again, even though they've already been audited individually. This is undesirable for a couple of reasons:
- the merge commit is usually large since it's a sum changes of all merged commits
- if a significant amount of time passes between individual changes being made and the merge, the auditor may spend a significant amount of time before even realizing they've already looked at the changes.
- it may hide actual real changes introduced in the merge (such as to resolve merge conflicts) that may be very valuable to review.
I would propose that the audit of merge commits specifically (commits with multiple parents commits) is generated slightly differently from the normal commit. Phabricator already detects merged commits for every merge commit and lists them in the commit/audit UI. If all of the commits being merged already have open audits, then the content of the commit for auditing can be presented as just the diff between the last merged commit and the merge commit itself (which would amount to the changes made during the merge, like conflict resolution). If those changes are empty (likely a majority of cases), the audit can be skipped. If not all of the commits being merged have audits (for example because they come from an untracked branch), then the merge commit would reflect the entirety of the changes.