| Summary: | EWS bubbles should indicate builder vs tester | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||||||
| Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | aakash_jain, ap, jbedard, ryanhaddad, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | Other | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=152790 https://bugs.webkit.org/show_bug.cgi?id=201532 https://bugs.webkit.org/show_bug.cgi?id=206807 |
||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Aakash Jain
2019-08-28 17:12:56 PDT
We should add these symbols to the tool-tip of the bubbles. Adding these symbols to the bubble itself would significantly increase the width of the bubbles and bubbles won't fit on one line anymore. Created attachment 377515 [details]
Patch
(In reply to Aakash Jain from comment #3) > Created attachment 377515 [details] > Patch Deployed on UAT instance. For most of the Apple queues, it's easy to determine builder vs tester just by the name of the queue. For other queues where the queue name doesn't contain builder or tester, we determine based on build-steps. e.g.: https://ews.webkit-uat.org/status-bubble/373911/ https://ews.webkit-uat.org/status-bubble/373459/ https://ews.webkit-uat.org/status-bubble/372833/ (In reply to Aakash Jain from comment #2) > We should add these symbols to the tool-tip of the bubbles. Adding these > symbols to the bubble itself would significantly increase the width of the > bubbles and bubbles won't fit on one line anymore. Would it Do we have a mandate to fit these bubbles in one line? Given how slow the tooltips are, I would personally find the characters more useful in the bubbles. Comment on attachment 377515 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377515&action=review > Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:56 > + BUILD_STEPS = ['compiled webkit'] Any reason this isn't just: ['compiled', 'build'] instead of being specific to WebKit? > Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:57 > + TEST_STEPS = ['api-tests', 'bindings-tests', 'layout-tests', 'webkitperl-tests', 'webkitpy tests'] Any reason this isn't just 'tests' instead of being specific to test suites? I don't know why we're so worried about bubble space. The whole bug page is so wasteful of space, we could easily redesign to allocate more space to EWS bubbles. Created attachment 377531 [details]
Empty space on this page
True, but the main concern is the review page. (In reply to Alexey Proskuryakov from comment #9) > True, but the main concern is the review page. I rarely consult the bubbles in the review page. How else do you know what EWS said as a reviewer? I think that bubbles on the review page are actually more important, as that's what the reviewer sees. (In reply to Simon Fraser (smfr) from comment #7) > I don't know why we're so worried about bubble space. ok, let's add these symbols/icons to the bubbles itself. (In reply to Alexey Proskuryakov from comment #9) > True, but the main concern is the review page. We can display these symbols/icons only for the bugs page, not for review page. That should solve both the concerns. I would need to re-write the logic. Since we would be displaying the symbols in the bubbles, we should display them all the time and irrespective of the build status (rather than trying to determine dynamically from the build, which the current patch tries to do). We would need a static mapping. Created attachment 377972 [details]
Patch
Comment on attachment 377972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377972&action=review > Tools/BuildSlaveSupport/ews-app/ews/common/buildbot.py:99 > + Buildbot.icons_for_queues_mapping[shortname] = icon Nit: I don't think the extra 'icon' variable is helpful here. Created attachment 377985 [details]
Patch
> Nit: I don't think the extra 'icon' variable is helpful here.
Removed in updated patch.
Also fixed the unit-test.
Committed r249483: <https://trac.webkit.org/changeset/249483> Deployed this change in production. Note that the review/detail page might show these symbols as well if the browser loads the cached version of the page. |