[webkit-dev] New EWS status bubbles in Bugzilla

[Alexey Proskuryakov]

29 сент. 2014 г., в 23:12, Carlos Garcia Campos <carlosgc at webkit.org> написал(а):

>> Please try it out, and let me know if something breaks, or is not as
>> good as it could be!
> 
> Looks great! thanks. I've noticed that now we use red also for patches
> that don't apply. I liked that there was a different color for that
> case, since it's common when a patch depends on another one, and the
> patch itself is not necessarily bad.

I agree that this is a non-obvious case. I chose red because EWS can't say anything good about the patch (or anything at all), so it needs extra careful attention from a reviewer.

Other options that I considered were:

- Keep it purple, as it was. I restricted purple to indicating internal errors, so it should pretty much never happen. Using the same color for the entirely different case of a patch that does not apply seems wrong.

- Make it white. That seems to make sense logically, but it has the same problem of using a single color for multiple purposes as red does. Also, I expect it to be surprising (I posted my patch several hours ago, why is it still white?)

- Add a new color. I'm not sure if this case is common enough for the color to become generally recognizable.

- Instead of per-queue bubbles, show text like "Patch does not apply to trunk". Not workable, because queues start at different times, and only some of them could fail to apply the patch.

Perhaps most importantly, this case is really no different from failing to build due to a dependency on another patch, or to failing some tests for the same reason. So I don't think that we can represent dependency on another patch with color.

Thinking about this now, we could replace bubbles with text when no queue has results (i.e. at least one queue has failed to apply, and those that didn't still haven't started processing - we can reasonably expect that they will fail, too, and even if they apply cleanly, we can just revert to showing bubbles then). I can't think of any situation where this would be misleading or difficult to comprehend. Would this resolve your concern? Would you be willing to post a patch implementing this?

- Alexey