Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2036773002:
Add new parameter --project_root to cpplint.py. (Closed)
|
Created:
4 years, 6 months ago by sdefresne Modified:
4 years, 5 months ago CC:
chromium-reviews, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org, iannucci, skym Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
DescriptionAdd new parameter --project_root to cpplint.py.
Chrome on iOS downstream repository tracks Chromium via DEPS and wants
to use cpplint canned presubmit check but the cpplint.py errors due at
the include guard as it stops at the inner most git repository.
Add a new parameter --project_root to cpplint.py. If set, it overrides
the automatic detection of the project root that searches the root of
the version control repository.
The --root parameter cannot be used as it is used after the automatic
resolution is performed (and allow chopping the head of the relative
path while the need is to expend the path to include ios_internal/).
BUG=598090
Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/263e92872fd2b9f3aedf3d368f48badc5bb3100f
Patch Set 1 #
Total comments: 4
Patch Set 2 : Address comments #Messages
Total messages: 27 (9 generated)
sdefresne@chromium.org changed reviewers: + iannucci@chromium.org, skym@chromium.org
This is another go at https://codereview.chromium.org/1897153003 but this time using an extra parameter not used by any project so I should not break anyone. Please take a look.
https://chromiumcodereview.appspot.com/2036773002/diff/1/cpplint.py File cpplint.py (right): https://chromiumcodereview.appspot.com/2036773002/diff/1/cpplint.py#newcode529 cpplint.py:529: _project_root = None I'm assuming that this is supposed to be an absolute path? Can we comment that here and assert it below, if true? https://chromiumcodereview.appspot.com/2036773002/diff/1/cpplint.py#newcode6023 cpplint.py:6023: (opts, filenames) = getopt.getopt(args, '', ['help', 'output=', 'verbose=', Is this script really using getopt? wtf? Who thought that was a good idea? :(
PTAL https://chromiumcodereview.appspot.com/2036773002/diff/1/cpplint.py File cpplint.py (right): https://chromiumcodereview.appspot.com/2036773002/diff/1/cpplint.py#newcode529 cpplint.py:529: _project_root = None On 2016/06/03 19:23:09, iannucci wrote: > I'm assuming that this is supposed to be an absolute path? Can we comment that > here and assert it below, if true? Done. https://chromiumcodereview.appspot.com/2036773002/diff/1/cpplint.py#newcode6023 cpplint.py:6023: (opts, filenames) = getopt.getopt(args, '', ['help', 'output=', 'verbose=', On 2016/06/03 19:23:09, iannucci wrote: > Is this script really using getopt? wtf? Who thought that was a good idea? :( Apparently :-( I'd say that fixing this is outside of the scope of that CL though :-)
Ping?
Ping. This is blocking enabling PRESUBMIT check in Chrome on iOS downstream repository.
Ping.
Ping. This is blocking adding some canned PRESUBMIT checks to the Chrome on iOS downstream repository and cause some style regression in the code. Can you please take a look or if you do not have time, can you suggest a better reviewer?
Ping. This is blocking adding some canned PRESUBMIT checks to the Chrome on iOS downstream repository and cause some style regression in the code. Can you please take a look or if you do not have time, can you suggest a better reviewer?
Ping?
sdefresne@chromium.org changed reviewers: + justincohen@chromium.org - iannucci@chromium.org, skym@chromium.org
Justin: can you take a look?
justincohen@chromium.org changed reviewers: + iannucci@chromium.org
justincohen@chromium.org changed required reviewers: + iannucci@chromium.org
lgtm
On 2016/07/11 14:15:14, justincohen wrote: > lgtm iannucci: can you do OWNERS review?
iannucci: ping
Description was changed from ========== Add new parameter --project_root to cpplint.py. Chrome on iOS downstream repository tracks Chromium via DEPS and wants to use cpplint canned presubmit check but the cpplint.py errors due at the include guard as it stops at the inner most git repository. Add a new parameter --project_root to cpplint.py. If set, it overrides the automatic detection of the project root that searches the root of the version control repository. The --root parameter cannot be used as it is used after the automatic resolution is performed (and allow chopping the head of the relative path while the need is to expend the path to include ios_internal/). BUG=598090 ========== to ========== Add new parameter --project_root to cpplint.py. Chrome on iOS downstream repository tracks Chromium via DEPS and wants to use cpplint canned presubmit check but the cpplint.py errors due at the include guard as it stops at the inner most git repository. Add a new parameter --project_root to cpplint.py. If set, it overrides the automatic detection of the project root that searches the root of the version control repository. The --root parameter cannot be used as it is used after the automatic resolution is performed (and allow chopping the head of the relative path while the need is to expend the path to include ios_internal/). BUG=598090 ==========
sdefresne@chromium.org changed reviewers: + dpranke@chromium.org
sdefresne@chromium.org changed required reviewers: + dpranke@chromium.org - iannucci@chromium.org
Dirk, can you do OWNERS review?
Stamp lgtm, though I'm a poor reviewer for this file. That said, the choice of getopt and globals over argparse and a cpplint object is pretty bad :/. If we have implicitly forked this from some upstream source, that's also pretty bad :(
lgtm
The CQ bit was checked by sdefresne@chromium.org
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.
Description was changed from ========== Add new parameter --project_root to cpplint.py. Chrome on iOS downstream repository tracks Chromium via DEPS and wants to use cpplint canned presubmit check but the cpplint.py errors due at the include guard as it stops at the inner most git repository. Add a new parameter --project_root to cpplint.py. If set, it overrides the automatic detection of the project root that searches the root of the version control repository. The --root parameter cannot be used as it is used after the automatic resolution is performed (and allow chopping the head of the relative path while the need is to expend the path to include ios_internal/). BUG=598090 ========== to ========== Add new parameter --project_root to cpplint.py. Chrome on iOS downstream repository tracks Chromium via DEPS and wants to use cpplint canned presubmit check but the cpplint.py errors due at the include guard as it stops at the inner most git repository. Add a new parameter --project_root to cpplint.py. If set, it overrides the automatic detection of the project root that searches the root of the version control repository. The --root parameter cannot be used as it is used after the automatic resolution is performed (and allow chopping the head of the relative path while the need is to expend the path to include ios_internal/). BUG=598090 Committed: https://chromium.googlesource.com/chromium/tools/depot_tools/+/263e92872fd2b9... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/tools/depot_tools/+/263e92872fd2b9... |
