| Summary: | [ews-app] Generate status-bubble hover-over messages | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||
| Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aakash_jain, ap, commit-queue, dewei_zhu, jbedard, lforschler, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Other | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Aakash Jain
2019-03-13 08:49:50 PDT
Created attachment 364540 [details] Proposed patch Tested on a local instance. Also replaced double-quotes(") with single-quotes('). This patch would apply to ToT after https://bugs.webkit.org/show_bug.cgi?id=195668 Comment on attachment 364540 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=364540&action=review > Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:67 > + bubble['state'] = 'pass' Is this correct? When we have warnings, we mark it as pass? If so, why not combine with the SUCCESS state? Comment on attachment 364540 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=364540&action=review > Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:68 > + bubble['details_message'] = 'Pass\n\n' + self._iso_time(build.complete_at) Maybe this should say 'Warning\n\n' instead of 'Pass\n\n', that way we're at least surfacing the error in some way. Comment on attachment 364540 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=364540&action=review >> Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:67 >> + bubble['state'] = 'pass' > > Is this correct? When we have warnings, we mark it as pass? If so, why not combine with the SUCCESS state? Yeah, because mostly build should finish with warning state in case of a minor issue. Any major issue with patch will fail the build. >> Tools/BuildSlaveSupport/ews-app/ews/views/statusbubble.py:68 >> + bubble['details_message'] = 'Pass\n\n' + self._iso_time(build.complete_at) > > Maybe this should say 'Warning\n\n' instead of 'Pass\n\n', that way we're at least surfacing the error in some way. Agree. will update it. Created attachment 364556 [details] Proposed patch > Maybe this should say 'Warning\n\n' instead of 'Pass\n\n', that way we're at least surfacing the error in some way. Done. Comment on attachment 364556 [details] Proposed patch Clearing flags on attachment: 364556 Committed r242967: <https://trac.webkit.org/changeset/242967> All reviewed patches have been landed. Closing bug. |