Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2251403002:
[MD History] Use modified time to check staleness of vulcanize and crisper. (Closed)
|
Created:
4 years, 4 months ago by calamity Modified:
4 years, 4 months ago CC:
chromium-reviews, Patrick Dubroy, michaelpg+watch-md-ui_chromium.org, dbeam+watch-history_chromium.org, pam+watch_chromium.org, arv+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[MD History] Use modified time to check staleness of vulcanize and crisper.
This CL changes the MD History presubmit script to check the modified
time to check if vulcanize needs to be run. This will throw up some false
positives but should be safer overall.
BUG=629406
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/ecf295d7196a5865ce7edeb753dff19acf83db69
Cr-Commit-Position: refs/heads/master@{#413063}
Patch Set 1 #
Total comments: 2
Patch Set 2 : fix nit #Messages
Total messages: 20 (10 generated)
Description was changed from ========== [MD History] Use modified time to check staleness of vulcanize and crisper. This CL changes the MD History presubmit script to check the modified time to check if vulcanize needs to be run. This will throw up some false positives but should be safer overall. BUG=629406 ========== to ========== [MD History] Use modified time to check staleness of vulcanize and crisper. This CL changes the MD History presubmit script to check the modified time to check if vulcanize needs to be run. This will throw up some false positives but should be safer overall. BUG=629406 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
calamity@chromium.org changed reviewers: + tsergeant@chromium.org
hey, cool! I like slowly reimplementing build systems as well!
lgtm https://codereview.chromium.org/2251403002/diff/1/chrome/browser/resources/md... File chrome/browser/resources/md_history/PRESUBMIT.py (right): https://codereview.chromium.org/2251403002/diff/1/chrome/browser/resources/md... chrome/browser/resources/md_history/PRESUBMIT.py:5: import os.path, time Nit: imports on separate lines
The CQ bit was checked by calamity@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...
https://codereview.chromium.org/2251403002/diff/1/chrome/browser/resources/md... File chrome/browser/resources/md_history/PRESUBMIT.py (right): https://codereview.chromium.org/2251403002/diff/1/chrome/browser/resources/md... chrome/browser/resources/md_history/PRESUBMIT.py:5: import os.path, time On 2016/08/19 00:24:37, tsergeant wrote: > Nit: imports on separate lines Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by calamity@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tsergeant@chromium.org Link to the patchset: https://codereview.chromium.org/2251403002/#ps20001 (title: "fix nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [MD History] Use modified time to check staleness of vulcanize and crisper. This CL changes the MD History presubmit script to check the modified time to check if vulcanize needs to be run. This will throw up some false positives but should be safer overall. BUG=629406 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== [MD History] Use modified time to check staleness of vulcanize and crisper. This CL changes the MD History presubmit script to check the modified time to check if vulcanize needs to be run. This will throw up some false positives but should be safer overall. BUG=629406 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/ecf295d7196a5865ce7edeb753dff19acf83db69 Cr-Commit-Position: refs/heads/master@{#413063} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ecf295d7196a5865ce7edeb753dff19acf83db69 Cr-Commit-Position: refs/heads/master@{#413063}
Message was sent while issue was closed.
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
Message was sent while issue was closed.
doing this for history is cool but you know what's cooler? doing it for both history AND downloads, cuz they're both like, vulcanized you know.
Message was sent while issue was closed.
On 2016/08/19 06:30:10, Dan Beam wrote: > doing this for history is cool > > but you know what's cooler? > > doing it for both history AND downloads, cuz they're both like, vulcanized you > know. I'll get around to making a more proper presubmit script that just runs vulcanize and checks if anything would have changed. This is a more temporary solution.
Message was sent while issue was closed.
On 2016/08/19 12:59:27, calamity wrote: > On 2016/08/19 06:30:10, Dan Beam wrote: > > doing this for history is cool > > > > but you know what's cooler? > > > > doing it for both history AND downloads, cuz they're both like, vulcanized you > > know. > > I'll get around to making a more proper presubmit script that just runs > vulcanize and checks if anything would have changed. This is a more temporary > solution. yeah, that's not a bad idea as well. and if you really want to improve vulcanize, maybe consider not writing the vulcanized files unless they differ so rebuilding is a no-op? |
