[Webkit-unassigned] [Bug 135104] Commit Queue clears the review flag, making it hard to tell whether a patch was reviewed and by whom

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=135104


Csaba Osztrogonác <ossy at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ossy at webkit.org




--- Comment #1 from Csaba Osztrogonác <ossy at webkit.org>  2014-07-21 02:28:12 PST ---
(In reply to comment #0)
> When Commit Queue commits a patch, it clears the r+ flag from the patch. Then to see which patches in a bug were reviewed and by whom, one must (a) find the patch’s attachment ID (b) navigate to the bug’s History and (c) look for history entries related to the attachment ID found in step (a).

I can't see the conrete issue you would like to solve here. 
Could you specify the details please?

If one follows the normal development work-flow, it shouldn't be a problem.
There is only one patch in one bug report (and maybe more r- patches until
the final r+ ) If the commit queue or somebody else lands the final r+ -ed
patch with "webkit-patch land /land-from-bug / land-attachemnt", the name
of the reviewer is added to the changelog and commit log too. In this case
there is no need to check the bug's history to determine who was the reviewer.

If we leave the r+ flags untouched, it can cause many problems in the future:
- After a rollout the bug will be reopened. In this case the remaining r+
is confusing, because this patch shouldn't be landed again and it appears
on http://webkit.org/pending-review , but it shouldn't.
- After r+ with comments, folks usually do two different things:
--- Land manually after review comments addressed.
--- Upload a patch with fixes + "Reviewed by X.Y." changelog entry + cq+ flag
- Sometimes folks upload different patches to one bug report. In this
case webkit-patch won't be able to handle more r+ flags properly.

I can imagine an improvement which can make folks and scripts happy too.
What do you think if CQ landed a patch, it would remove the r+ flag
and change the "patch" / "proposed patch" / ... description to
"landed in r123456, r=X.Y." ?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.