Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2925993002:
Polymer: Remove <paper-material> from polymer_resources.grdp file. (Closed)
DescriptionPolymer: Remove <paper-material> from polymer_resources.grdp file.
BUG=598516
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2925993002
Cr-Commit-Position: refs/heads/master@{#477770}
Committed: https://chromium.googlesource.com/chromium/src/+/93bd3b4b2f3930c82b0287b2f475594a8947f898
Patch Set 1 #
Total comments: 3
Patch Set 2 : Undo python change #
Total comments: 1
Messages
Total messages: 21 (15 generated)
Description was changed from ========== Polymer; Remove <paper-material> from polymer_resounces.grdp file. BUG=598516 ========== to ========== Polymer; Remove <paper-material> from polymer_resounces.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by dpapad@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 ========== Polymer; Remove <paper-material> from polymer_resounces.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Polymer: Remove <paper-material> from polymer_resounces.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Polymer: Remove <paper-material> from polymer_resounces.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Polymer: Remove <paper-material> from polymer_resources.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hey, I know this wasn't sent for review yet but wanted to understand what steps we need to add. https://codereview.chromium.org/2925993002/diff/1/tools/polymer/polymer_grdp_... File tools/polymer/polymer_grdp_to_txt.py (right): https://codereview.chromium.org/2925993002/diff/1/tools/polymer/polymer_grdp_... tools/polymer/polymer_grdp_to_txt.py:17: # Following elements are not used anymore, but are unfortunately downloaded Can you clarify what your purpose is in adding this check? The grdp file is not auto-generated from third_party/polymer, so it looks like you're trying to avoid human error. This just removes the blacklisted files from the text output when I run polymer_grdp_to_txt.py. It doesn't prevent me from adding the files to the .grdp directly. Nor does it prevent me from adding these files when I do this (this is how I normally add elements btw): 1. polymer_grdp_to_txt.py ui/webui/resources/polymer_resources.grdp > elements.txt 2. vim elements.txt # add/remove what I want 3. txt_to_polymer_grdp.py > ui/webui/resources/polymer_resources.grdp
https://codereview.chromium.org/2925993002/diff/1/tools/polymer/polymer_grdp_... File tools/polymer/polymer_grdp_to_txt.py (right): https://codereview.chromium.org/2925993002/diff/1/tools/polymer/polymer_grdp_... tools/polymer/polymer_grdp_to_txt.py:17: # Following elements are not used anymore, but are unfortunately downloaded On 2017/06/07 at 05:44:07, michaelpg wrote: > Can you clarify what your purpose is in adding this check? The grdp file is not auto-generated from third_party/polymer, so it looks like you're trying to avoid human error. > > This just removes the blacklisted files from the text output when I run polymer_grdp_to_txt.py. It doesn't prevent me from adding the files to the .grdp directly. > > Nor does it prevent me from adding these files when I do this (this is how I normally add elements btw): > > 1. polymer_grdp_to_txt.py ui/webui/resources/polymer_resources.grdp > elements.txt > 2. vim elements.txt # add/remove what I want > 3. txt_to_polymer_grdp.py > ui/webui/resources/polymer_resources.grdp What I had in mind was that if someone runs 1,2,3 without doing any manual editing, they would get a grdp file that does not have any obsoleted elements. To be honest, I don't understand why any manual editing is needed, and consequently why the grdp file is not automatically updated as part of reproduce.sh. Adding to my confusion, the instructions about steps 1,2,3 are only displayed within polymer_resources.grdp, which means that whoever runs reproduce.sh needs to know beforehand that they also need to update that file. I would expect reproduce.sh to at least print instructions in the output of manual steps need to be taken by someone rolling Polymer elements.
The CQ bit was checked by dpapad@chromium.org to run a CQ dry run
dpapad@chromium.org changed reviewers: + michaelpg@chromium.org
https://codereview.chromium.org/2925993002/diff/1/tools/polymer/polymer_grdp_... File tools/polymer/polymer_grdp_to_txt.py (right): https://codereview.chromium.org/2925993002/diff/1/tools/polymer/polymer_grdp_... tools/polymer/polymer_grdp_to_txt.py:17: # Following elements are not used anymore, but are unfortunately downloaded > To be honest, I don't understand why any manual editing is needed, and consequently why the grdp file is not automatically updated as part of reproduce.sh. Adding to my confusion, the instructions about steps 1,2,3 are only displayed within polymer_resources.grdp, which means that whoever runs reproduce.sh needs to know beforehand that they also need to update that file. I would expect reproduce.sh to at least print instructions in the output of manual steps need to be taken by someone rolling Polymer elements. Resolved per offline discussion with michaelpg@, by reverting the change to this file. https://codereview.chromium.org/2925993002/diff/20001/third_party/polymer/v1_... File third_party/polymer/v1_0/reproduce.sh (left): https://codereview.chromium.org/2925993002/diff/20001/third_party/polymer/v1_... third_party/polymer/v1_0/reproduce.sh:24: check_dep "which polymer-css-build" "polymer-css-build" \ I am also removing this since it is a left-over from when reproduce.sh was re-building all vulcanized checked in files.
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: CQ cannot get SignCLA result. Please try later.
lgtm
The CQ bit was checked by dpapad@chromium.org
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": 20001, "attempt_start_ts": 1496873873316780,
"parent_rev": "650b5a81d0cad5ce038e1dc16c70ee267645b6ba", "commit_rev":
"93bd3b4b2f3930c82b0287b2f475594a8947f898"}
Message was sent while issue was closed.
Description was changed from ========== Polymer: Remove <paper-material> from polymer_resources.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Polymer: Remove <paper-material> from polymer_resources.grdp file. BUG=598516 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2925993002 Cr-Commit-Position: refs/heads/master@{#477770} Committed: https://chromium.googlesource.com/chromium/src/+/93bd3b4b2f3930c82b0287b2f475... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/93bd3b4b2f3930c82b0287b2f475... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
