Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2089223003:
annotations: change meaning of STEP_TEXT (Closed)
Descriptionannotations: change meaning of STEP_TEXT
recipe engine prints new lines in step text by putting <br/>; it abuses the fact that buildbot
prints step text as HTML without escaping it. Milo escapes step text and
as a result we see <br/> in step text.
Change buildbot so that messages specified in STEP_TEXT annotation mean
lines, not strings that are joined by space.
Next CLs will change recipe engine to not print <br> and change milo to
put <br> between step text strings.
R=martiniss@chromium.org, hinoka@chromium.org
BUG=622503
Committed: https://chromium.googlesource.com/chromium/tools/build/+/cfb258c35d2cfca391c6580acaaad2da08d10a3d
Patch Set 1 #
Total comments: 4
Patch Set 2 : remove step name from step text #Messages
Total messages: 15 (4 generated)
Description was changed from ========== annotations: change meaning of STEP_TEXT recipe engine prints new line sin step text by abusing the fact that buildbot prints step text as HTML without escaping it. Milo escapes step text and as a result we see <br/> in step text. Change buildbot so that messages specified in STEP_TEXT annotation mean lines, not strings that are joined by space. Next CLs will change recipe engine to not print <br> and change milo to put <br> between step text strings. R=martiniss@chromium.org, hinoka@chromium.org BUG=622503 ========== to ========== annotations: change meaning of STEP_TEXT recipe engine prints new lines in step text by putting <br/>; it abuses the fact that buildbot prints step text as HTML without escaping it. Milo escapes step text and as a result we see <br/> in step text. Change buildbot so that messages specified in STEP_TEXT annotation mean lines, not strings that are joined by space. Next CLs will change recipe engine to not print <br> and change milo to put <br> between step text strings. R=martiniss@chromium.org, hinoka@chromium.org BUG=622503 ==========
PTAL
Preview: http://imgur.com/lYMfRo2
hinoka@google.com changed reviewers: + hinoka@google.com
https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... File scripts/master/chromium_step.py (right): https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... scripts/master/chromium_step.py:50: step_text = ['<br>%s' % line for line in section['step_text']] I think we the first line of step_text to be on the same line, right?
https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... File scripts/master/chromium_step.py (right): https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... scripts/master/chromium_step.py:50: step_text = ['<br>%s' % line for line in section['step_text']] On 2016/06/22 22:44:24, Ryan Tseng wrote: > I think we the first line of step_text to be on the same line, right? Dunno. Note that it will get mixed with step name. It was always confusing to me, why does buildbot display "<step name again> <step text>" on the same line. we could remove step name too. wdyt?
https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... File scripts/master/chromium_step.py (right): https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... scripts/master/chromium_step.py:50: step_text = ['<br>%s' % line for line in section['step_text']] On 2016/06/22 22:47:21, nodir wrote: > On 2016/06/22 22:44:24, Ryan Tseng wrote: > > I think we the first line of step_text to be on the same line, right? > > Dunno. Note that it will get mixed with step name. It was always confusing to > me, why does buildbot display "<step name again> <step text>" on the same line. > > we could remove step name too. wdyt? yeah removing step name sounds good to me, i'm savvy.
https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... File scripts/master/chromium_step.py (right): https://codereview.chromium.org/2089223003/diff/1/scripts/master/chromium_ste... scripts/master/chromium_step.py:50: step_text = ['<br>%s' % line for line in section['step_text']] On 2016/06/22 22:50:08, Ryan Tseng wrote: > On 2016/06/22 22:47:21, nodir wrote: > > On 2016/06/22 22:44:24, Ryan Tseng wrote: > > > I think we the first line of step_text to be on the same line, right? > > > > Dunno. Note that it will get mixed with step name. It was always confusing to > > me, why does buildbot display "<step name again> <step text>" on the same > line. > > > > we could remove step name too. wdyt? > > yeah removing step name sounds good to me, i'm savvy. Done. http://imgur.com/BWUWRbH
lgtm Much cleaner! imo the step text doesn't really need to be on a new line for the "running recipe <xyz>", but up to you.
On 2016/06/22 22:59:29, Ryan Tseng wrote: > lgtm > > Much cleaner! > > imo the step text doesn't really need to be on a new line for the "running > recipe <xyz>", but up to you. i am not sure what you mean. if there is only one string in step_text, it will be on the same line
On 2016/06/22 22:59:29, Ryan Tseng wrote: > lgtm > > Much cleaner! > > imo the step text doesn't really need to be on a new line for the "running > recipe <xyz>", but up to you. the screenshot was made with unpatched recipe engine which emits <br>, that's why there is unnecessary line there there won't be a new line with new recipe engine
The CQ bit was checked by nodir@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2089223003/20001
Message was sent while issue was closed.
Description was changed from ========== annotations: change meaning of STEP_TEXT recipe engine prints new lines in step text by putting <br/>; it abuses the fact that buildbot prints step text as HTML without escaping it. Milo escapes step text and as a result we see <br/> in step text. Change buildbot so that messages specified in STEP_TEXT annotation mean lines, not strings that are joined by space. Next CLs will change recipe engine to not print <br> and change milo to put <br> between step text strings. R=martiniss@chromium.org, hinoka@chromium.org BUG=622503 ========== to ========== annotations: change meaning of STEP_TEXT recipe engine prints new lines in step text by putting <br/>; it abuses the fact that buildbot prints step text as HTML without escaping it. Milo escapes step text and as a result we see <br/> in step text. Change buildbot so that messages specified in STEP_TEXT annotation mean lines, not strings that are joined by space. Next CLs will change recipe engine to not print <br> and change milo to put <br> between step text strings. R=martiniss@chromium.org, hinoka@chromium.org BUG=622503 Committed: https://chromium.googlesource.com/chromium/tools/build/+/cfb258c35d2cfca391c6... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/tools/build/+/cfb258c35d2cfca391c6... |
