Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1253173007:
SoM: fix bug links (Closed)
|
Created:
5 years, 4 months ago by seanmccullough Modified:
5 years, 4 months ago CC:
chromium-reviews, johnme Base URL:
https://chromium.googlesource.com/infra/infra.git@master Target Ref:
refs/heads/master Project:
infra Visibility:
Public. |
DescriptionSoM: fix bug links
BUG=515855
Committed: https://chromium.googlesource.com/infra/infra/+/09b9ea60d7da2a018a2f1cf75aa00b1f4cc56847
Patch Set 1 #
Total comments: 2
Patch Set 2 : https #Messages
Total messages: 20 (7 generated)
seanmccullough@chromium.org changed reviewers: + jshu@chromium.org
ping
On 2015/08/03 at 16:47:17, seanmccullough wrote: > ping lgtm
On 2015/08/03 at 17:01:07, jshu wrote: > On 2015/08/03 at 16:47:17, seanmccullough wrote: > > ping > > lgtm Might actually be better to insert this.bug = this.bug[0] after Line 74 model/ct-failure-group.html since we don't need the bug attribute to store an array in the first place
On 2015/08/03 17:03:50, jshu wrote: > On 2015/08/03 at 17:01:07, jshu wrote: > > On 2015/08/03 at 16:47:17, seanmccullough wrote: > > > ping > > > > lgtm > > Might actually be better to insert this.bug = this.bug[0] after Line 74 > model/ct-failure-group.html since we don't need the bug attribute to store an > array in the first place In principle I agree. However, model/ct-failure-group does stuff with _annotate, .data and bug that turns bug into an array for some reason I haven't found the time to grok. Changing .bug to something besides an array risks breaking code elsewhere that assumes it is an array.
The CQ bit was checked by seanmccullough@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1253173007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1253173007/1
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
seanmccullough@chromium.org changed reviewers: + stip@chromium.org - jshu@chromium.org
On 2015/08/03 at 17:26:52, seanmccullough wrote:
>
this.bug = /([0-9]{3,})/.exec(this._annotation.bug);
is why the bug is an array. that exec function returns an array. it is on line
72 of ct-failure-group.
pretty confident from global search changing bug wouldn't break code anywhere
else.
stip@chromium.org changed reviewers: + jshu@google.com
lgtm with https! https://chromiumcodereview.appspot.com/1253173007/diff/1/appengine/sheriff_o_... File appengine/sheriff_o_matic/ui/ct-failure-stream.html (right): https://chromiumcodereview.appspot.com/1253173007/diff/1/appengine/sheriff_o_... appengine/sheriff_o_matic/ui/ct-failure-stream.html:97: return `http://crbug/${bug[0]}`; https! https://en.wikipedia.org/wiki/HTTPS_Everywhere
https://chromiumcodereview.appspot.com/1253173007/diff/1/appengine/sheriff_o_... File appengine/sheriff_o_matic/ui/ct-failure-stream.html (right): https://chromiumcodereview.appspot.com/1253173007/diff/1/appengine/sheriff_o_... appengine/sheriff_o_matic/ui/ct-failure-stream.html:97: return `http://crbug/${bug[0]}`; On 2015/08/03 17:34:02, stip wrote: > https! > > https://en.wikipedia.org/wiki/HTTPS_Everywhere crbug redirects anyways, but yeah :)
The CQ bit was checked by seanmccullough@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stip@chromium.org, jshu@google.com Link to the patchset: https://chromiumcodereview.appspot.com/1253173007/#ps20001 (title: "https")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1253173007/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1253173007/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/infra/infra/+/09b9ea60d7da2a018a2f1cf75aa00... |
