input_api.basename is a function, so printing it out yields:
"The <function basename at 0x7fccd3735410> directory requires..>"
Instead, use it to resolve the local presubmit path that has
requested clang formatting.
R=dpranke@chromium.org
BUG=422332
Review URL: https://codereview.chromium.org/665493002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@292491 0039d316-1c4b-4281-b951-d872f2087c98
If presubmit.py for a directory (such as net/) fails due to not being clang formatted, then 'git cl format' needs to be run for that directory, not the entire patch.
BUG=422332
Review URL: https://codereview.chromium.org/648713003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@292407 0039d316-1c4b-4281-b951-d872f2087c98
This check never triggered before, since cpplint.py was looking for
override not OVERRIDE. Since Chromium prefers override now, the
linter is now (correctly) warning. However, the old Chromium convention
(and the one implemented by the clang plugin) is to explicitly annotate
all overrides with both virtual and override. The clang plugin code has
been updated, but new binaries have not yet been built. Until clang
rolls, suppress the warning.
BUG=417463
TBR=stip@chromium.org
Review URL: https://codereview.chromium.org/627163002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@292319 0039d316-1c4b-4281-b951-d872f2087c98
'Add a description' is not enough to know what the test is about. Added
some more info.
BUG=
Review URL: https://codereview.chromium.org/313523002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@274393 0039d316-1c4b-4281-b951-d872f2087c98
This method is necessary to allow good code organization (i.e., unittests live
adjacent to the source files they test) in the new infra.git repository.
R=maruel@chromium.org
Review URL: https://codereview.chromium.org/281013002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@270564 0039d316-1c4b-4281-b951-d872f2087c98
I want to run Python unit tests as part of my presubmit, and those unit tests require PYTHONPATH to be set the way it would be set when the modules they test are running.
Review URL: https://codereview.chromium.org/250693002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@265998 0039d316-1c4b-4281-b951-d872f2087c98
2006-2009/10 was a valid copyright date format before 2011 (when we decided to
use only one year, not a range).
The current directive with copyright is in fact to not change old copyright
dates, so there is no plan to go back and "fix" old files. As such, the
presubmit checks should not be complaining about old files.
BUG=none
Review URL: https://codereview.chromium.org/200643002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@257181 0039d316-1c4b-4281-b951-d872f2087c98
For simplicity, the canned presubmit check just calls git cl format
directly with a new option --dry-run which does not change the files on
disk.
Because some users may not have git or clang-format might somehow have
errors, this canned check is a warning. Additionally, if git cl format
fails with an error then the presubmit check will silently report
success to avoid spamming misconfigured users during upload.
BUG=none
Review URL: https://codereview.chromium.org/141493002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@246066 0039d316-1c4b-4281-b951-d872f2087c98
This is useful to diagnose slow tests. Make this multiprocessing aware. Stop
printing when the commands are added but use the proper message system instead.
R=iannucci@chromium.org
BUG=
Review URL: https://codereview.chromium.org/99453003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@238388 0039d316-1c4b-4281-b951-d872f2087c98
I've unified the checks for language specific exceptions to line length. There is a behavior change where cpp compiler directives no longer have a maximum line length (previously they were restricted to 150% of max line length). This change was made to match the behavior for java files.
BUG=NONE
Review URL: https://codereview.chromium.org/100163002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@238268 0039d316-1c4b-4281-b951-d872f2087c98
Previously we would suggest owners for the whole CL, rather than just
the files currently missing owner approval.
R=maruel@chromium.org
BUG=130978
Review URL: https://chromiumcodereview.appspot.com/23591064
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@224183 0039d316-1c4b-4281-b951-d872f2087c98
Currently the output is:
Found line ending with white spaces in:
cc/trees/layer_tree_host_impl_unittest.cc, line 379
This format is extremely annoying because you must combine the
file and line number together by hand. A more standard format
for expressing a position in a file is filename:lineno. This
changes the format to match:
Found line ending with white spaces in:
cc/trees/layer_tree_host_impl_unittest.cc:379
R=maruel@chromium.org
BUG=
Review URL: https://chromiumcodereview.appspot.com/22837004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@217405 0039d316-1c4b-4281-b951-d872f2087c98
Greatly improves presubmit performance on Blink where there may be rule violations (my test case went from 50s to 5s).
BUG=236206
TEST=new test in presubmit_unittest.py; manual performance test on Mac and Windows after touching hundreds of files in webkit; testing included add/move/edit/delete on both binary and text files.
Review URL: https://chromiumcodereview.appspot.com/15898005
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@205275 0039d316-1c4b-4281-b951-d872f2087c98
This will let clients that don't use the heavily filtered canned cpplint
check and want to behave more strictly (aka cc/) reuse any common
settings applied to the cpplint module.
R=maruel@chromium.org
BUG=none
Review URL: https://codereview.chromium.org/13866044
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@194264 0039d316-1c4b-4281-b951-d872f2087c98
For hash_map and hash_set, we should include "base/hashtables.h", not
the <hash_set> or <hash_map> standard headers.
R=maruel@chromium.org
BUG=
Review URL: https://chromiumcodereview.appspot.com/14139002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@194047 0039d316-1c4b-4281-b951-d872f2087c98
Blink wants to override the maxlen which is currently configurable in CheckLongLines but not in PanProjectChecks. This exposes the parameter on PanProjectChecks.
Review URL: https://chromiumcodereview.appspot.com/13403004
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@192394 0039d316-1c4b-4281-b951-d872f2087c98
If we uploaded a CL w/o any reviewers specified, and then
ran the owners check on it (so that we had a Rietveld
issue number) we would crash in a Python assertion. This
wasn't caught in unit testing because of a limitation
in the unit test scaffolding.
TBR=maruel@chromium.org
BUG=None
Review URL: https://chromiumcodereview.appspot.com/12546003
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@186454 0039d316-1c4b-4281-b951-d872f2087c98
Until an issue is uploaded to Rietveld, we don't know the official
email address to use for an owners check. There are three ways to fix
this: we could attempt to log in to rietveld prior to doing the check
and extract the address to use, or we could use ~/.last_codereview_email_address,
or we can use the email address we can determine from the checkout.
All three options have flaws; the first is particularly awkward since there
doesn't seem to be a good way to fetch the email without posting an issue.
The second is flawed if we use different addresses for different repos,
and the third is flawed if the checkout's email address is different from
the rietveld address, or if it is anonymous.
However, since this is only being used for owners checks (in this case),
anonymous checkouts probably don't matter, and hopefully the cases where
the email addresses differ are rare.
R=maruel@chromium.org
BUG=118388, 150049
Review URL: https://chromiumcodereview.appspot.com/12377023
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@186259 0039d316-1c4b-4281-b951-d872f2087c98
Right now we require approval from someone, and we require an owner
approval, but we don't require an approval from an owner *other than
the patch other*. It's conceivable that we might want this, so
I am making this a configurable argument to the presubmit check.
This will also be needed to ensure that we don't suggest you as an
owner for your own patches, when we actually know who you are.
R=maruel@chromium.org
BUG=
Review URL: https://chromiumcodereview.appspot.com/12326151
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@185294 0039d316-1c4b-4281-b951-d872f2087c98
Currently, when we run the OWNERS check, we print the list of directories
that contain the relevant OWNERS files for any modified files in a change
still needing approval.
This has two problems:
1) if we bubble all the way up to the top level OWNERS, we print "" instead of
"src/" or something more useful (bug 157191)
2) for OWNERS files that contain per-file set-noparent entries (like changes to IPC messages), this can be really confusing because an owner of other stuff in the directory might've approved things already.
This change will now print the list of files in the CL that are still unapproved.
This might be a lot more verbose (since you get N lines rather than 1 for N files in a given directory), but hopefully it'll be clearer in the two cases above.
Also, this change takes care of some lingering clean-up in the code to rename some methods to be clearer.
R=maruel@chromium.org
BUG=157191
Review URL: https://chromiumcodereview.appspot.com/12314044
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@184219 0039d316-1c4b-4281-b951-d872f2087c98
Printing the list of all the files examined can produce a lot
out of output and bury the interesting output.
BUG=
Review URL: https://chromiumcodereview.appspot.com/12315031
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@183813 0039d316-1c4b-4281-b951-d872f2087c98
We were passing a unicode string in the env block to subprocess.Popen which makes it unhappy. This forces the string to ascii first.
BUG=NONE
TEST=NONE
Review URL: https://chromiumcodereview.appspot.com/12255083
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@183569 0039d316-1c4b-4281-b951-d872f2087c98
The Android WebView code includes several manually-maintained .mk files
that are included by the Android build system. These contain some fairly
long lines as they need to refer to deep pathnames, which means the CQ
cannot be used as it always triggers a presubmit warning.
Allow .mk files to have 200 columns per line instead, which should be
enough for the long paths that are used. Add a test case for this.
BUG=
Review URL: https://chromiumcodereview.appspot.com/12252067
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@182733 0039d316-1c4b-4281-b951-d872f2087c98
r176777 introduced a change that relied on os.path.samepath, which does not exist on Windows.
BUG=
Review URL: https://chromiumcodereview.appspot.com/12021013
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@177701 0039d316-1c4b-4281-b951-d872f2087c98
pylint checks were erronously excluded for PRESUBMIT.py files in
root directory of repos because the regex was getting a './' added.
R=iannucci@chromium.org
TBR=M-A
Review URL: https://chromiumcodereview.appspot.com/11880040
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@176777 0039d316-1c4b-4281-b951-d872f2087c98
This fixes a potential issue with RunPylint in windows where paths
can contain special characters.
TBR=iannucci@chromium.org
Review URL: https://chromiumcodereview.appspot.com/11786007
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@175481 0039d316-1c4b-4281-b951-d872f2087c98
The AffectedSourceFiles query matches against files with paths relative
to repository root. On the other hand _FindAllFiles checks white_lists
and black_lists against files paths relative to PRESUBMIT.py
As a result, when a PRESUBMIT.py is not at RepositoryRoot, RunPylint
can misbehave.
I fix this by modifying the white_lists and black_list regexes to
include the relative directory of PRESUBMIT.py.
Also adding some logging to pylint and simplified misc logic.
Review URL: https://chromiumcodereview.appspot.com/11776016
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@175364 0039d316-1c4b-4281-b951-d872f2087c98
We use the fact the dcommit adds a "Committed:" link to the
description. If we wanted to be more strict, we could make
git cl dcommit post a comment on the CL (which would be nice
since it would notify reviewers that the CL was committed).
Note that this is untested as written, because it doesn't appear
that we have either git-cl integration tests, nor a test for
git cl dcommit adding the word "Committed:" so as written, this
presubmit check is prone to failure. Would like suggestions to
improve.
Also adding DoNotSubmit checks and a couple others to
PanPresubmitChecks.
R=maruel@chromium.org,dpranke@chromium.org
BUG=161702
Review URL: https://chromiumcodereview.appspot.com/11421050
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@171158 0039d316-1c4b-4281-b951-d872f2087c98
This can come up when CQ closed an issue but the user's
local branch is still tied to the issue.
BUG=161702
Review URL: https://chromiumcodereview.appspot.com/11348122
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@171153 0039d316-1c4b-4281-b951-d872f2087c98
on all modified files, and don't require (c).
BUG=140977
Review URL: https://chromiumcodereview.appspot.com/10824191
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@150417 0039d316-1c4b-4281-b951-d872f2087c98
First, the environment variable for the child process was created but not
specified to subprocess.call().
Second, third_party/logilab/__init__.py tried to initialize itself with
pkg_resources.
TBR=chrisha@chromium.org
BUG=
TEST=
Review URL: https://chromiumcodereview.appspot.com/10582031
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@143111 0039d316-1c4b-4281-b951-d872f2087c98
This CL adds pylint (and its dependencies) to third_party. It plumbs them into presubmit_canned_checks, and exposes a command-line tool to access pylint.
BUG=
TEST=
Review URL: https://chromiumcodereview.appspot.com/10447014
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@143016 0039d316-1c4b-4281-b951-d872f2087c98
BUG=None
TEST=Change files that need OWNERS review. Upload the patch. Check that the warning suggests a minimum set of reviewers.
Review URL: https://chromiumcodereview.appspot.com/10222020
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@135619 0039d316-1c4b-4281-b951-d872f2087c98
Update pylintrc to be near the pylint 0.25.1 default file.
R=cmp@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/10199016
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@133707 0039d316-1c4b-4281-b951-d872f2087c98
Chrome for Android introduced the requirement for java files.
Android's style guide:
http://source.android.com/source/code-style.html#limit-line-length
defines 100 columns for .java files.
This patch changes the presubmit tests which are also used by chromium's CQ.
TEST=testCannedCheckJavaLongLines
Review URL: http://codereview.chromium.org/9417023
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@122938 0039d316-1c4b-4281-b951-d872f2087c98
this forces license headers, no line-end whitespace, etc.
Also update the default license header regex to allow concluding */ on the final line.
Review URL: http://codereview.chromium.org/9109001
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@119448 0039d316-1c4b-4281-b951-d872f2087c98
Instead of crashing, prints an error message.
BUG=
TEST=
Review URL: http://codereview.chromium.org/8363030
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@106867 0039d316-1c4b-4281-b951-d872f2087c98
Add a warning when RunUnitTestsInDirectory() finds no test to run to
catch this kind of regression.
Fix all the regressions that where introduced in the meantime...
TBR=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/7906009
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@101347 0039d316-1c4b-4281-b951-d872f2087c98
stderr is not redirected by default. stdout is not allowed.
Both were oversight.
Do not override stdin=None in case the user would response to stderr output for
example.
Increase test coverage.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/7860041
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@100456 0039d316-1c4b-4281-b951-d872f2087c98
It result in less code, forces uploading the change for eventual review and most
importantly enables the commit queue to correctly handle this.
BUG=
TEST=
Review URL: http://codereview.chromium.org/7253015
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@91575 0039d316-1c4b-4281-b951-d872f2087c98
Improve the list of exception to include very_long_symbol_names.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/7097012
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@89696 0039d316-1c4b-4281-b951-d872f2087c98
This makes it possible to dump logging information from a PRESUBMIT script when
calling git cl presubmit -v -v -v for example.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/7027031
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@87603 0039d316-1c4b-4281-b951-d872f2087c98
This makes it coherent with the commit queue by keeping the lgtm at a single
place.
This simplifies owners check code.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/7082029
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@87251 0039d316-1c4b-4281-b951-d872f2087c98
The presubmit check complained about long lines in .grd files. This isn't a useful warning since .grd files aren't really source files.
The problem was that callers were passing in a file filter to InputApi.AffectedFiles but it was being ignored. Fix was to use the passed in file filter.
BUG=None
TEST=Ran on a local change and verified that I no longer got long line warnings on .grd files.
Review URL: http://codereview.chromium.org/6932060
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@85148 0039d316-1c4b-4281-b951-d872f2087c98
This check is also handled by the commit bot so no need to harass people when
uploading.
R=dpranke@chromium.org
BUG=none
TEST=Nobody wrote a test for PanProjectChecks() and I didn't write this function, no way I won't.
Review URL: http://codereview.chromium.org/6998008
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@84814 0039d316-1c4b-4281-b951-d872f2087c98
The commit bot automagically fixes these issues so the devs don't need to fix these problems
TEST=none
BUG=none
R=dpranke@chromium.org
Review URL: http://codereview.chromium.org/6948006
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@84572 0039d316-1c4b-4281-b951-d872f2087c98
seems to take an inordinate amount of time. The idea is to
lint whole files, and only try to compute deltas if the file
contains at least one error.
For my environment, the three affected presubmit rules took
2000ms each for a 4-file changelist. With this change,
all my rules run in about a second, and none take anything
close to 500ms.
Also, since there seem to be environment-dependent factors at
work here, I'm proposing putting timer warnings that print
a message if any check takes longer than 500ms.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=82559
Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=82568
Review URL: http://codereview.chromium.org/6883050
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@82607 0039d316-1c4b-4281-b951-d872f2087c98
seems to take an inordinate amount of time. The idea is to
lint whole files, and only try to compute deltas if the file
contains at least one error.
For my environment, the three affected presubmit rules took
2000ms each for a 4-file changelist. With this change,
all my rules run in about a second, and none take anything
close to 500ms.
Also, since there seem to be environment-dependent factors at
work here, I'm proposing putting timer warnings that print
a message if any check takes longer than 500ms.
Review URL: http://codereview.chromium.org/6883050
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@82559 0039d316-1c4b-4281-b951-d872f2087c98
It could result in failed tests so I'll revert if it ever becomes problematic.
It ran fine with my limited testing.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/6813114
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@81279 0039d316-1c4b-4281-b951-d872f2087c98
Add propagation of verbose flag from git cl to presubmit_support
Rename NotImplementedException to standard NotImplementError.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/6810012
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80798 0039d316-1c4b-4281-b951-d872f2087c98
The reason of this change (beside deleting 30 lines) is to fix pylint on Mac.
For an unknown reason, it was dying here trying to process the minidom import.
R=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/6799021
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80770 0039d316-1c4b-4281-b951-d872f2087c98
Deprecate RunPythonUnitTests() since it is a bit awkward.
Add os_listdir, os_walk and platform to InputApi.
GOAL=Slowly adds python 2.5 support for unit tests
BUG=none
TEST=new unit test
R=dpranke@chromium.org
Review URL: http://codereview.chromium.org/6778027
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@80009 0039d316-1c4b-4281-b951-d872f2087c98
encountering Foundation types that are not id-derived, including NSPoint,
NSRange, NSRect, and NSSize.
BUG=77438
TEST=Modify chrome/browser/renderer_host/render_widget_host_view_mac.mm
Review URL: http://codereview.chromium.org/6755007
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@79564 0039d316-1c4b-4281-b951-d872f2087c98
This fixes bugs 76724. We will now allow approvals from
only non-owners to be sufficient if the patch is from an OWNER.
Also, this strips out the code for suggesting reviewers during upload completely. I've come to believe that this should be done by gcl and git-cl directly rather than through a presubmit hook, when the change is being initially created. CheckOwners() is now a no-op on upload.
(We might need to add a new value to the codereview.settings file instead to indicate if we want this to happen).
Review URL: http://codereview.chromium.org/6730020
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@79339 0039d316-1c4b-4281-b951-d872f2087c98
should never import any modules directly but always access modules
through input_api.
Review URL: http://codereview.chromium.org/6676128
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@79337 0039d316-1c4b-4281-b951-d872f2087c98
It also simplifies all of the PresubmitResult* objects and the way we're using output streams since we can assume more about how the callers are calling us.
Note that I uses PEP-8 style method names where I was adding entirely new classes (or rewriting existing classes completely) since per maruel@ he is trying to new that style for new code.
This change also contains two small changes to git_cl to fix bugs and restore the previous behavior of --force skipping the presubmit checks.
Review URL: http://codereview.chromium.org/6694009
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@78328 0039d316-1c4b-4281-b951-d872f2087c98
This change requires us to change the previous signature for the CheckOwners() hook to provide the server address and email regexp to use for parsing approvals from Rietveld.
Review URL: http://codereview.chromium.org/6657028
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@77891 0039d316-1c4b-4281-b951-d872f2087c98