Dear All, This is a RFC after discussions with Ben Cooksley, who as always was truly helpful (thanks!). Sending it here since the proposal affects global behaviour of KDE git.
tl;dr I propose to treat public KDE git branches (for all repos) having '-work' suffix in a special way: ignore REVIEW and BUG/CCBUG lines. When commit hits a public KDE git branch without -work suffix, current behaviour is preserved. ** The problem: Whenever commits are pushed from work branches and they contain merged/cherry-picked commits (from other developers) with REVIEW: line, we're getting repeating notes on rb and emails: "This review has been submitted with commit xxxxxxxxx by yyyyyyyy to branch zzzzz." I think the hook shall not generate duplicated reviews like this. Moreover the issue propagates to bugs.kde.org: multiple commits with the same BUG: or CCBUG: line also cause multiple events registered within the bug entry, and multiple email notifications. The 'phenomena' was reported at https://bugs.kde.org/show_bug.cgi?id=309307 **Question: Could it be possible in the commit hook to block further reviews for submitted reviews? This is not a discussion about bug in reviewboard software but about extending the commit hooks. **What workflow is affected: Affected workflow that use integration with git.reviewboard.kde.org and/or bugs.kde.org is as follows: 1. Push a change without a REVIEW but optionally with BUG/CCBUG lines. Of course because we do not know the review number at the moment). Pushing to a public work branch is often needed if we expect someone will need/want to test the code. The patch depends on other patches, so the full context is needed in the public (and reviewboard needs the context otherwise it refuses uploads of patches --you might have noticed this) If the BUG line was used - the bug gets marked as 'resolved' too early, so we already have a problem. Followers receive false information via email notification; and if this happens the and the committer knows about the misfeature, she can reopen the bug and another confusing notification is sent to followers. -- this is the problem #1 2. Post the patch to the reviewboard. We're getting the review number. 3. Carefully copy the number and paste it into altered commit message for the patch. Git changes the commit hash. 4. Push the change to the same public work branch. We do not want to create separate local branch (and recompile!) just because the message changed, and we of course want to keep the branch synced with the KDE git server for our workgroup - others might want to push here too. 5. Once we pushed to the work branch, the review gets 'submitted' status since there's REVIEW line - wrong - we might not get any review yet. -- this is again another instance of the problem #1 6. Once we get a Ship It! review we're cherry-picking this particular reviewed patch to target branches (to more than one). Merge is not universal unless the feature being worked on is put within one commit. It's quite often not. For example work on it can be splited into more parts, some released in x.1 minor version and another released in another minor version. Some can have positive review, some not. For example there are fixes to given feature that I am pushing to Calligra 2.5, 2.6 and master branches. Result? 4 messages about submitted patch (from work branch, from 2.5, 2.6 and from master) are created. -- this is the problem #2 **Proposed solution: With support from Ben I know detecting duplicated reviews is not practical or reliable. And it only addresses the problem #2. So I propose to treat public KDE git branches (for all repos) with '-work' suffix in a special way: ignore REVIEW and BUG/CCBUG lines. When commit hits a public KDE git branch without -work suffix, current behaviour is preserved. **Notes: 1. I know if merge is used, there's no problem with duplicated commits. However as mentioned in 6. above, if the code is splited to orthogonal parts, e.g. core change and cosmetic fixes (comments/dead code removal, what sometimes improves readability of the patch for review), cherry-pick is handy, but results in duplicates git commit hashes in the repo. Blocking REVIEW and BUG/CCBUG when merging is used effectively disables them, and since each commit (with given hash) hits the server only once, and it's within the work branch. So if someone plans to use merge-based workflow, -work suffix should not be used for name of the branch. 2. The proposal isn't about hiding kde-commits emails. Commit filters can be used to reduce number of emails. -- regards / pozdrawiam, Jaroslaw Staniek Kexi & Calligra & KDE | http://calligra.org/kexi | http://kde.org Qt Certified Specialist | http://qt-project.org http://www.linkedin.com/in/jstaniek