Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1982423002:
Get rid of LASTCHANGE.blink (Closed)
DescriptionGet rid of LASTCHANGE.blink
Now that blink lives in the src repo, there's no need to generate a
separate LASTCHANGE file for it.
The LASTCHANGE line makes it into the user agent. LASTCHANGE.blink
used --git-hash-only to only have the git hash in there. Remove
that now-unused flag and use version.py's -e flag to get the same
effect for webkit_version.h
Reverts parts of https://chromiumcodereview.appspot.com/14973005/
No intended behavior change.
BUG=none
Review-Url: https://codereview.chromium.org/1982423002
Cr-Commit-Position: refs/heads/master@{#465739}
Committed: https://chromium.googlesource.com/chromium/src/+/69e2570d4b5e50cd708e4c467e567d810c4b2134
Patch Set 1 #
Total comments: 2
Patch Set 2 : rebase #
Total comments: 1
Patch Set 3 : rebase #Patch Set 4 : rebase #Patch Set 5 : try to fix devtools #
Messages
Total messages: 31 (17 generated)
thakis@chromium.org changed reviewers: + dpranke@chromium.org
scottmg@chromium.org changed reviewers: + scottmg@chromium.org
I have no idea what this one is, but maybe remove the .blink here too https://code.google.com/p/chromium/codesearch#chromium/tools/chrome-devtools-... https://codereview.chromium.org/1982423002/diff/1/tools/export_tarball/export... File tools/export_tarball/export_tarball.py (right): https://codereview.chromium.org/1982423002/diff/1/tools/export_tarball/export... tools/export_tarball/export_tarball.py:149: # This command is from src/DEPS; please keep them in sync. keep them -> keep it
https://codereview.chromium.org/1982423002/diff/1/tools/export_tarball/export... File tools/export_tarball/export_tarball.py (right): https://codereview.chromium.org/1982423002/diff/1/tools/export_tarball/export... tools/export_tarball/export_tarball.py:149: # This command is from src/DEPS; please keep them in sync. On 2016/05/17 17:19:27, scottmg wrote: > keep them -> keep it Actually, 'keep them' is better (I interpreted referents incorrectly). Ignore me.
On 2016/05/17 17:19:27, scottmg wrote: > I have no idea what this one is, but maybe remove the .blink here too > > https://code.google.com/p/chromium/codesearch#chromium/tools/chrome-devtools-... That looks like some bisection script thingy, and for older revisions the file is still needed, so we should probably keep it there for a few months more after this CL here lands. I can add a TODO there (but not in this CL; that file is in a different repo).
Description was changed from ========== Get rid of LASTCHANGE.blink Now that blink lives in the src repo, there's no need to generate a separate LASTCHANGE file for it. Reverts parts of https://chromiumcodereview.appspot.com/14973005/ No intended behavior change. BUG=none ========== to ========== Get rid of LASTCHANGE.blink Now that blink lives in the src repo, there's no need to generate a separate LASTCHANGE file for it. Reverts parts of https://chromiumcodereview.appspot.com/14973005/ No intended behavior change. BUG=none ==========
On 2016/05/17 17:44:32, Nico (vacation May 19 to 22) wrote: > On 2016/05/17 17:19:27, scottmg wrote: > > I have no idea what this one is, but maybe remove the .blink here too > > > > > https://code.google.com/p/chromium/codesearch#chromium/tools/chrome-devtools-... > > That looks like some bisection script thingy, and for older revisions the file > is still needed, so we should probably keep it there for a few months more after > this CL here lands. I can add a TODO there (but not in this CL; that file is in > a different repo). That line creates LASTCHANGE.blink because it is used by "webkit_version" action which is involved in building "devtools_frontend_resources" target. You can coordinate with dgozman@ rolling in your change. I guess what can be done before committing it is updating "uploader_iteration.sh" to create both "LASTCHANGE" and "LASTCHANGE.blink", and after some time the latter can be eliminated. The "bisection thing" doesn't use LASTCHANGE.blink, it just finds the last commit for which DevTools FE hasn't been pushed to the cloud yet.
lgtm assuming mnaganov/dgozman/pfeldman approve. https://codereview.chromium.org/1982423002/diff/20001/build/util/lastchange.py File build/util/lastchange.py (right): https://codereview.chromium.org/1982423002/diff/20001/build/util/lastchange.p... build/util/lastchange.py:182: FetchGitRevision(directory, hash_only)) Ideally we'd get rid of all of the SVN and Git+SVN logic and just look for git commit id's, but that seems like a separate CL.
I've committed https://codereview.chromium.org/1985973005/ to accommodate for this change, and it looks like it cycled successfully. Please proceed and thanks for spotting this out! lgtm
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1982423002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1982423002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
mnaganov / dgozman: Do you happen to know why this would make a bunch of devtools browser_tests fail? (eg https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...)
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/05/18 17:45:09, Nico wrote: > mnaganov / dgozman: Do you happen to know why this would make a bunch of > devtools browser_tests fail? (eg > https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...) Looks like the issue is that LASTCHANGE.blink is generated with --git-hash-only, and so LASTCHANGE.blink looks like `LASTCHANGE=e6974751597de203bb5a78dc82fa253517338f0e` while LASTCHANGE looks like `LASTCHANGE=e6974751597de203bb5a78dc82fa253517338f0e-refs/heads/master@{#463254}` -- and that '-refs/heads/master' then confuses webkit_version.h generation (which devtools depends on).
The CQ bit was checked by thakis@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Get rid of LASTCHANGE.blink Now that blink lives in the src repo, there's no need to generate a separate LASTCHANGE file for it. Reverts parts of https://chromiumcodereview.appspot.com/14973005/ No intended behavior change. BUG=none ========== to ========== Get rid of LASTCHANGE.blink Now that blink lives in the src repo, there's no need to generate a separate LASTCHANGE file for it. The LASTCHANGE line makes it into the user agent. LASTCHANGE.blink used --git-hash-only to only have the git hash in there. Remove that now-unused flag and use version.py's -e flag to get the same effect for webkit_version.h Reverts parts of https://chromiumcodereview.appspot.com/14973005/ No intended behavior change. BUG=none ==========
dgozman: I think I finally figured out what was going wrong, see diff from patch
set 4 to 5. Namely, LASTCHANGE looks like
LASTCHANGE=7c9bd5f0202de1500d75c22654b2e22f0bc34312-refs/heads/master@{#465654}
while LASTCHANGE.blink looked like
LASTCHANGE=7c9bd5f0202de1500d75c22654b2e22f0bc34312
I modified the version.py invocation to strip off stuff starting at '-' -- you
might have to change uploader_iteration.sh to make sure your LASTCHANGE also
contains a - and some stuff behind it.
The CQ bit was unchecked by thakis@chromium.org
The CQ bit was checked by thakis@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org, dgozman@chromium.org Link to the patchset: https://codereview.chromium.org/1982423002/#ps80001 (title: "try to fix devtools")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1492633526651370,
"parent_rev": "054d4e484b45c4c7872eec3459ccb1c5f85a3b1e", "commit_rev":
"69e2570d4b5e50cd708e4c467e567d810c4b2134"}
Message was sent while issue was closed.
Description was changed from ========== Get rid of LASTCHANGE.blink Now that blink lives in the src repo, there's no need to generate a separate LASTCHANGE file for it. The LASTCHANGE line makes it into the user agent. LASTCHANGE.blink used --git-hash-only to only have the git hash in there. Remove that now-unused flag and use version.py's -e flag to get the same effect for webkit_version.h Reverts parts of https://chromiumcodereview.appspot.com/14973005/ No intended behavior change. BUG=none ========== to ========== Get rid of LASTCHANGE.blink Now that blink lives in the src repo, there's no need to generate a separate LASTCHANGE file for it. The LASTCHANGE line makes it into the user agent. LASTCHANGE.blink used --git-hash-only to only have the git hash in there. Remove that now-unused flag and use version.py's -e flag to get the same effect for webkit_version.h Reverts parts of https://chromiumcodereview.appspot.com/14973005/ No intended behavior change. BUG=none Review-Url: https://codereview.chromium.org/1982423002 Cr-Commit-Position: refs/heads/master@{#465739} Committed: https://chromium.googlesource.com/chromium/src/+/69e2570d4b5e50cd708e4c467e56... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/69e2570d4b5e50cd708e4c467e56... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
