[RFC] Solution for duplicated or false review/bug notifications

[Jaroslaw Staniek]

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