Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2609683002:
clang update script: change --tools to --extra-tools (Closed)
|
Created:
3 years, 11 months ago by dcheng Modified:
3 years, 11 months ago CC:
chromium-reviews, eugenis+clang_chromium.org, vmpstr+watch_chromium.org, Lei Zhang, dsinclair, yunlian, glider+clang_chromium.org, Nico, ukai+watch_chromium.org, Reid Kleckner, hans, dmikurube+clang_chromium.org, nasko Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionclang update script: add --extra-tools, deprecate --tools flag
plugins and blink_gc_plugin are always mandatory for Chromium builds,
so just always include them. In order to avoid breaking codesearch
bots, --tools is now a synonym for --extra-tools and will be removed.
BUG=none
Committed: https://crrev.com/f239071ad582021eb28bfa3ea1d9444d9a2e84f9
Cr-Commit-Position: refs/heads/master@{#441601}
Patch Set 1 #Patch Set 2 : . #
Total comments: 2
Patch Set 3 : . #
Messages
Total messages: 19 (9 generated)
dcheng@chromium.org changed reviewers: + danakj@chromium.org, lukasza@chromium.org
I got tired of typing plugins blink_gc_plugin every time.
LGTM with one nit. https://codereview.chromium.org/2609683002/diff/20001/docs/clang_tool_refacto... File docs/clang_tool_refactoring.md (right): https://codereview.chromium.org/2609683002/diff/20001/docs/clang_tool_refacto... docs/clang_tool_refactoring.md:106: and the [Blink to Chrome style rewriter](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/rewrite_to_chrome_style/). Additional arguments to `--extra-tools` should be the name of nit: wrap this to 80 columns?
thakis@chromium.org changed reviewers: + thakis@chromium.org
Is this going to break the codesearch bots? https://cs.chromium.org/search/?q=%5C--tools&sq=package:chromium&type=cs … (maybe add new flag, but accept old flag for a bit, then move those bots to the new flag, then remove old flag)
On 2016/12/31 00:49:45, Nico (offline until Jan 1) wrote: > Is this going to break the codesearch bots? > https://cs.chromium.org/search/?q=%5C--tools&sq=package:chromium&type=cs … > (maybe add new flag, but accept old flag for a bit, then move those bots to the > new flag, then remove old flag) Done -- I made --tools a synonym for now. https://codereview.chromium.org/2609683002/diff/20001/docs/clang_tool_refacto... File docs/clang_tool_refactoring.md (right): https://codereview.chromium.org/2609683002/diff/20001/docs/clang_tool_refacto... docs/clang_tool_refactoring.md:106: and the [Blink to Chrome style rewriter](https://chromium.googlesource.com/chromium/src/+/master/tools/clang/rewrite_to_chrome_style/). Additional arguments to `--extra-tools` should be the name of On 2016/12/30 17:19:01, Łukasz Anforowicz wrote: > nit: wrap this to 80 columns? I'll try this in a followup: wrapping links is always awkward.
update cl description. with that, lgtm assuming the plan is to make the codesearch bots use the new flag and remove the old spelling, and assuming you watch the codesearch bots since they now build with the plugins while they weren't before
Description was changed from ========== clang update script: change --tools to --extra-tools plugins and blink_gc_plugin are always mandatory for Chromium builds, so just always include them. BUG=none ========== to ========== clang update script: add --extra-tools, deprecate --tools flag plugins and blink_gc_plugin are always mandatory for Chromium builds, so just always include them. In order to avoid breaking codesearch bots, --tools is now a synonym for --extra-tools and will be removed. BUG=none ==========
The CQ bit was checked by dcheng@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from lukasza@chromium.org Link to the patchset: https://codereview.chromium.org/2609683002/#ps40001 (title: ".")
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": 40001, "attempt_start_ts": 1483592984052240,
"parent_rev": "b47ad5ac79beaf26eef852c365efe9f3de67bae2", "commit_rev":
"32ef71ea223d32b09f16d3cbc4b8bc310037b584"}
Message was sent while issue was closed.
Description was changed from ========== clang update script: add --extra-tools, deprecate --tools flag plugins and blink_gc_plugin are always mandatory for Chromium builds, so just always include them. In order to avoid breaking codesearch bots, --tools is now a synonym for --extra-tools and will be removed. BUG=none ========== to ========== clang update script: add --extra-tools, deprecate --tools flag plugins and blink_gc_plugin are always mandatory for Chromium builds, so just always include them. In order to avoid breaking codesearch bots, --tools is now a synonym for --extra-tools and will be removed. BUG=none Review-Url: https://codereview.chromium.org/2609683002 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== clang update script: add --extra-tools, deprecate --tools flag plugins and blink_gc_plugin are always mandatory for Chromium builds, so just always include them. In order to avoid breaking codesearch bots, --tools is now a synonym for --extra-tools and will be removed. BUG=none Review-Url: https://codereview.chromium.org/2609683002 ========== to ========== clang update script: add --extra-tools, deprecate --tools flag plugins and blink_gc_plugin are always mandatory for Chromium builds, so just always include them. In order to avoid breaking codesearch bots, --tools is now a synonym for --extra-tools and will be removed. BUG=none Committed: https://crrev.com/f239071ad582021eb28bfa3ea1d9444d9a2e84f9 Cr-Commit-Position: refs/heads/master@{#441601} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/f239071ad582021eb28bfa3ea1d9444d9a2e84f9 Cr-Commit-Position: refs/heads/master@{#441601}
Message was sent while issue was closed.
alekseys@chromium.org changed reviewers: + alekseys@chromium.org
Message was sent while issue was closed.
This change makes one of the bots unhappy: https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT
Message was sent while issue was closed.
On 2017/01/06 01:43:45, Aleksey Shlyapnikov wrote: > This change makes one of the bots unhappy: > https://build.chromium.org/p/chromium.fyi/builders/CFI%20Linux%20ToT Sorry about the build break. I already have a fix in the CQ which should resolve this: https://codereview.chromium.org/2618633005/ |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
