Bug 28933 - webkit-patch post-commits should not remove r- flag.
Summary: webkit-patch post-commits should not remove r- flag.
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2009-09-02 22:34 PDT by Fumitoshi Ukai
Modified: 2010-01-19 17:36 PST (History)
2 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Fumitoshi Ukai 2009-09-02 22:34:56 PDT
bugzilla-tool shouldn't remove r- flag just because a new patch is posted.
Comment 1 Eric Seidel (no email) 2009-10-23 14:32:44 PDT
So it should only clear r? is that what you're asking?  But that it should leave r+ and r- flags set when obsoleting a patch?
Comment 2 Eric Seidel (no email) 2009-10-23 14:33:13 PDT
The code for this is all in http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/bugzilla.py
Comment 3 Eric Seidel (no email) 2010-01-19 16:45:40 PST
I think this was a misunderstanding more than a bug.  Please re-open if I've misunderstood further. :)
Comment 4 Alexey Proskuryakov 2010-01-19 16:58:49 PST
What kind of misunderstanding was this? Certainly r- shouldn't be cleared from obsolete patches, why would anyone want to lose this information?
Comment 5 Eric Seidel (no email) 2010-01-19 17:06:13 PST
We can't leave r+ on obsolete patches due to our inability to make a "patches to land" query which excludes obsolete patches.  See http://webkit.org/pending-commit

Why would we need to leave any r+/r-/r? information on obsolete patches?  That information is never lost.  Its easily viewable from the history if needed.  (e.g. https://bugs.webkit.org/show_activity.cgi?id=32920)  If you'd like, we could easily make the obsolete method in bugzilla.py (used by webkit-patch) make a comment about who had marked it r-.

It's an easy change to make it leave the r-, but I'm not sure it's much value.  Ideally we wouldn't be limited by bugzilla and we would just leave whatever flags we wanted on obsolete patches.  But unless we can fix bugzilla to filter them out of our "patches to review" and "patches to commit" queries, humans get confused when we leave these flags around.
Comment 6 Alexey Proskuryakov 2010-01-19 17:18:21 PST
I don't agree that bugzilla-tool behavior should deviate from default Bugzilla behavior. We are not all going to switch to bugzilla-tool - I, for one, find it inherently less usable. Why should I ever resort to a command line tool if I can just post a form from my browser?
Comment 7 Eric Seidel (no email) 2010-01-19 17:24:09 PST
You and I seem to be talking past one another.  I'll see if I can track you down on #webkit.
Comment 8 Eric Seidel (no email) 2010-01-19 17:36:52 PST
ap and I talked on IRC.

bugzilla already removes r? when obsoleting patches.
we have to remove r+ to make http://webkit.org/pending-commit behave nicely.
we could leave r- if we wanted to.

Leaving this open as nice-to-have.

The ideal fix would be to http://webkit.org/pending-commit to have a query which ignored r+ on obsolete patches, or to fix bugzilla to support such a query. :)