Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1761293002:
Use px in color-profile-{background-clip-text,svg-fill-text}.html (Closed)
|
Created:
4 years, 9 months ago by Noel Gordon Modified:
4 years, 9 months ago Reviewers:
Dirk Pranke CC:
chromium-reviews, blink-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUse px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737, 591901
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
Committed: https://crrev.com/685c9b8cdc54f0bac465fc5170a77c206c871510
Cr-Commit-Position: refs/heads/master@{#379440}
Patch Set 1 #Patch Set 2 : ToT sync, fix the presubmit lint. #
Messages
Total messages: 35 (17 generated)
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOTRY=true
==========
noel@chromium.org changed reviewers: + dpranke@chromium.org
The CQ bit was checked by noel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1761293002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1761293002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOTRY=true
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
==========
The CQ bit was checked by noel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1761293002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1761293002/1
Message was sent while issue was closed.
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4 Cr-Commit-Position: refs/heads/master@{#379253}
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/1764693005/ by treib@chromium.org. The reason for reverting is: Broke webkit_lint on all the bots. What was the justification for NOPRESUBMIT and NOTRY here?.
Message was sent while issue was closed.
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
Dinner and the fact that these test are being rebaselined. I failed to see the presubmit lint issue. Let's try it again.
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOPRESUBMIT=true
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
The CQ bit was checked by noel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1761293002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1761293002/20001
The CQ bit was unchecked by dpranke@chromium.org
un-cq'ing ... why are you sending this NOTRY=true TBR= ? There's no reason for either of those things, and this change could produce real failures that the tryjobs will catch.
On 2016/03/04 21:59:42, Dirk Pranke wrote: > un-cq'ing ... why are you sending this NOTRY=true TBR= ? There's no reason for > either of those things, and this change could produce real failures that the > tryjobs will catch. These are rebaselines. These test are not passing yet. Much more code need to land before they will pass. Please provide an LGTM foir these rebaselines if you prefer that.
lgtm. I still don't see a reason to skip the try jobs for this; the only normal reasons to do that are to fix build breakages or for things that due to bugs won't get through the normal process.
Any trys will skip these two tests since they have both been marked [ NeedsRebaseline ] on all ports, no?
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
TBR=dpranke@chromium.org
BUG=587737,591901
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
On 2016/03/04 22:24:45, noel gordon wrote: > Any trys will skip these two tests since they have both been marked [ > NeedsRebaseline ] on all ports, no? NeedsRebaseline doesn't actually skip tests, it just runs them and ignores the results (skipping them would mean that we'd never generate the new baselines we need). Regardless, in my view it's not a good idea to try and decide when you do or don't need to run try jobs and skip things; project policy (as I understand it), which I agree with, is to always run the try jobs unless you're in the situations I described above. You should not worry about saving resources, and there's rarely otherwise a reason to be in such a hurry that you can't wait the hour or two it'll take to land.
On 2016/03/04 22:38:19, Dirk Pranke wrote: > On 2016/03/04 22:24:45, noel gordon wrote: > > Any trys will skip these two tests since they have both been marked [ > > NeedsRebaseline ] on all ports, no? > > NeedsRebaseline doesn't actually skip tests, it just runs them and ignores the > results (skipping them would mean > that we'd never generate the new baselines we need). Yes, that is understanding too. > Regardless, in my view it's not a good idea to try and decide when you do or > don't need to run try jobs and skip > things; project policy (as I understand it), which I agree with, is to always > run the try jobs unless you're in the > situations I described above. You should not worry about saving resources, and My concern was about resource waste: needlessly using up the power, or generating green-house gases etc, for no good reason. If we should not care about that, then I certainly don't mind running try jobs as I would with a normal CL. This one felt more like Blink Gardening, where I would use a NOTRY submit. > there's rarely otherwise a reason > to be in such a hurry that you can't wait the hour or two it'll take to land. Or like it's my weekend already :)
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
NOTRY=true
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
The CQ bit was checked by noel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1761293002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1761293002/20001
On 2016/03/05 01:31:42, noel gordon wrote: > My concern was about resource waste: needlessly using up the power, or > generating green-house gases etc, for no good reason. > > If we should not care about that, then I certainly don't mind running try jobs > as I would with a normal CL. This one felt more like Blink Gardening, where I > would use a NOTRY submit. Yup, generally we'd rather people follow the normal process than try to save resources. We try to focus on saving resources elsewhere, by, e.g., running `analyze` to figure out we don't need to do anything, etc.
Message was sent while issue was closed.
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
==========
to
==========
Use px in color-profile-{background-clip-text,svg-fill-text}.html
Use px for font size and positions in these tests to produce more
consistent rendering across ports (issue 591901), and request new
test rebaselines on all ports.
BUG=587737,591901
Committed: https://crrev.com/5982e29a45b6d2074d85daa86409c6065adfd8f4
Cr-Commit-Position: refs/heads/master@{#379253}
Committed: https://crrev.com/685c9b8cdc54f0bac465fc5170a77c206c871510
Cr-Commit-Position: refs/heads/master@{#379440}
==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/685c9b8cdc54f0bac465fc5170a77c206c871510 Cr-Commit-Position: refs/heads/master@{#379440} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
