WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
137290
EWS is too eager to say that a patch does not apply to trunk of repository
https://bugs.webkit.org/show_bug.cgi?id=137290
Summary
EWS is too eager to say that a patch does not apply to trunk of repository
Alexey Proskuryakov
Reported
2014-10-01 00:05:57 PDT
The patch in
bug 137272
was processed by some queues, yet EWS claims that it was not.
Attachments
proposed fix
(4.32 KB, patch)
2014-10-01 00:08 PDT
,
Alexey Proskuryakov
rniwa
: review+
Details
Formatted Diff
Diff
better fix
(4.48 KB, patch)
2014-10-01 09:07 PDT
,
Alexey Proskuryakov
rniwa
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2014-10-01 00:08:28 PDT
Created
attachment 239012
[details]
proposed fix
Ryosuke Niwa
Comment 2
2014-10-01 06:40:52 PDT
Comment on
attachment 239012
[details]
proposed fix View in context:
https://bugs.webkit.org/attachment.cgi?id=239012&action=review
> Tools/QueueStatusServer/handlers/statusbubble.py:165 > + bubble["had_resultative_status_other_than_failure_to_apply"] = any(map(lambda status: status.message not in progress_statuses and status.message != "Patch does not apply" and status.message != "Error: " + queue.name() + " unable to apply patch.", statuses))
Can we split this into two lines? It's awfully long. Alternatively, I would the variable name to did_not_fail_to_apply.
> Tools/QueueStatusServer/handlers/statusbubble.py:191 > + had_resultative_status_other_than_failure_to_apply = any(map(lambda bubble: bubble["had_resultative_status_other_than_failure_to_apply"] == True, bubbles))
No need to compare with True (i.e. remove == True).
Alexey Proskuryakov
Comment 3
2014-10-01 09:07:10 PDT
Created
attachment 239029
[details]
better fix I realized that the truth lies in between - with the previous patch, we could get all red and all popups would just say "failed to apply", which is useless.
Ryosuke Niwa
Comment 4
2014-10-01 09:58:08 PDT
Comment on
attachment 239029
[details]
better fix View in context:
https://bugs.webkit.org/attachment.cgi?id=239029&action=review
> Tools/QueueStatusServer/handlers/statusbubble.py:129 > + bubble["had_resultative_status_other_than_failure_to_apply"] = any(map(lambda status: latest_resultative_status and latest_resultative_status.message != "Error: " + queue.name() + " unable to apply patch.", statuses))
Again, can we split this into two lines? It's really long.
Daniel Bates
Comment 5
2014-10-01 10:08:21 PDT
Comment on
attachment 239029
[details]
better fix View in context:
https://bugs.webkit.org/attachment.cgi?id=239029&action=review
> Tools/QueueStatusServer/app.yaml:2 > +version: ap # Bugzilla bug ID of last major change
I take it you are using ap as placeholder. Regardless, the inline comment doesn't make sense at least since <
http://trac.webkit.org/changeset/162288
>. We seem to be using the SVN revision number instead of the Bugzilla bug id for the version since <
http://trac.webkit.org/changeset/161062
>. This is error prone and has led to a mismatch between the actual SVN revision and the revision recorded in this file at least once so far in <
http://trac.webkit.org/changeset/162288
>. I assume that the actual value of version matters less than changing it to be something different from the last recorded version, possibly with the invariant that its value is monotonically increasing. Regardless, we should standardize a convention for updating it and update the comment in this file to document the convention.
Daniel Bates
Comment 6
2014-10-01 10:09:47 PDT
(In reply to
comment #5
)
> (From update of
attachment 239029
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=239029&action=review
> > > Tools/QueueStatusServer/app.yaml:2 > > +version: ap # Bugzilla bug ID of last major change > >... Regardless, the inline comment doesn't make sense at least since <
http://trac.webkit.org/changeset/162288
>....
*the inline comment doesn't make sense.
Alexey Proskuryakov
Comment 7
2014-10-01 10:18:29 PDT
Committed <
http://trac.webkit.org/r174158
>.
Alexey Proskuryakov
Comment 8
2014-10-01 10:24:16 PDT
> the inline comment doesn't make sense
I guess I never read the comment! I think that we always used svn revision here, Ryosuke guided me to do so when I first made a change to the webkit-queues app. Yes, I think that an svn revision is more useful than a bug ID, as we can bisect when something breaks (AppEngine keeps old versions available until one manually deletes them).
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug