Make committing to toolchain-utils easier |
||
Issue descriptionCommitting to toolchain-utils is a bit of a fragmented experience at the moment (we have some git hooks that're only run on direct pushes, and which depend on Google-internal code. We also don't have built-in pylint.) It would be nice if: - We ran our current git hooks as a part of `repo upload` - We use a Python formatter that's accessible from inside of the chroot (and, ideally, open source) - We ran linting as a part of our hooks This bug tracks that work.
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/0e74b9bf195b55935e81f79ffcd1382a9a678715 commit 0e74b9bf195b55935e81f79ffcd1382a9a678715 Author: George Burgess IV <gbiv@google.com> Date: Fri Jan 04 10:09:05 2019 toolchain-utils: add a top-level `yapf` file yapf is a Python autoformatter, much like how clang-tidy is an autoformatter for c/c++. It's available by default in the chroot, and appears to be in use by our autotest friends. I ran `yapf -i *.py`, and it only suggested a handful of changes from our current style here. The only one I really noticed was that it preferred 'foo %s' % (really_long_variable_name_one, really_long_variable_name_two) over 'foo %s' % (really_long_variable_name_one, really_long_variable_name_two) ...Which I think we do in some files, anyway. This file magically applies to all `yapf` invocations inside of toolchain-utils, so it's not something that people need to worry about. Simply run `yapf -i file.py`, and it'll be automatically formatted to conform to Chromium's style guidelines, which we try to follow here. BUG= chromium:918755 TEST=`yapf *.py` left files mostly untouched. `cros lint` is just as (un)happy after the format as it was before. Change-Id: Ie09694d8466b398ca07f5c8544597c9a28c7abfe Reviewed-on: https://chromium-review.googlesource.com/1392445 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Luis Lozano <llozano@chromium.org> [add] https://crrev.com/0e74b9bf195b55935e81f79ffcd1382a9a678715/.style.yapf
,
Jan 5
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/c100127d900d69608ab715ddcfac1d783c7848ef commit c100127d900d69608ab715ddcfac1d783c7848ef Author: George Burgess IV <gbiv@google.com> Date: Sat Jan 05 22:52:12 2019 toolchain-utils: swap git hooks to use yapf This updates a few things: - Migrates our git hooks to directly use yapf for formatting (which appears to be the direction that Chromium is heading in, and is available by default both inside and outside of the chroot: Chromium's depot_tools has a copy of it. pyformat is only available internally, and is basically a wrapper around yapf, except for a few things. Please see CL 1392445 for more), - Splits out the format checks into its own script, so it's easier to potentially run from e.g. repo (don't quote me on that quite yet, though ;) ) - Removes the now-seemingly-unused tc_format script BUG= chromium:918755 TEST=- ./check-format on a poorly formatted Python file - ./check-format on a python file with a #! that shouldn't have had it - ./check-format on a python file without a #! that should have had it - ./check-format on toolchain-utils/*.py; results were overall reasonable All fixes it suggested worked as intended. CQ-DEPEND=CL:1392445 Change-Id: Idabc2658c5dedcc70b6d3cc6c1acfd6c83eabc29 Reviewed-on: https://chromium-review.googlesource.com/1394285 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Manoj Gupta <manojgupta@chromium.org> Reviewed-by: Luis Lozano <llozano@chromium.org> [modify] https://crrev.com/c100127d900d69608ab715ddcfac1d783c7848ef/toolchain_utils_githooks/pre-push.real [add] https://crrev.com/c100127d900d69608ab715ddcfac1d783c7848ef/toolchain_utils_githooks/check-format [delete] https://crrev.com/0e74b9bf195b55935e81f79ffcd1382a9a678715/bin/tc_pyformat
,
Jan 6
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/e136e0a53ae27ca7ff6ad31e6b4f86fde7238410 commit e136e0a53ae27ca7ff6ad31e6b4f86fde7238410 Author: George Burgess IV <gbiv@google.com> Date: Sun Jan 06 06:44:00 2019 toolchain-utils: add a `cros lint` git hook This adds a git hook to run `cros lint` on all of the files we're committing. It also adds a wrapper around both check-lint and check-format that runs the two in parallel without making the output interleave. Better names for check-style are appreciated. BUG= chromium:918755 TEST=Ran ./check-style on a few Python files. Got the expected output/lint errors/etc. Works both outside and inside of the chroot. CQ-DEPEND=CL:1394285 Change-Id: I30c74662759f27abdfd663083d04002a05260bf7 Reviewed-on: https://chromium-review.googlesource.com/1395807 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Manoj Gupta <manojgupta@chromium.org> [modify] https://crrev.com/e136e0a53ae27ca7ff6ad31e6b4f86fde7238410/toolchain_utils_githooks/pre-push.real [add] https://crrev.com/e136e0a53ae27ca7ff6ad31e6b4f86fde7238410/toolchain_utils_githooks/check-style [add] https://crrev.com/e136e0a53ae27ca7ff6ad31e6b4f86fde7238410/toolchain_utils_githooks/check-lint
,
Jan 12
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/34b4506112b52a32844bcababd96a894f1cae0c4 commit 34b4506112b52a32844bcababd96a894f1cae0c4 Author: George Burgess IV <gbiv@google.com> Date: Sat Jan 12 22:04:58 2019 toolchain-utils: make repo do presubmit checks This CL makes repo do our Python style checks on `repo upload`. BUG= chromium:918755 TEST=`repo upload .` with broken Python Change-Id: I2744637a5c4c1b25f11f56354ef39e04fd2a100b Reviewed-on: https://chromium-review.googlesource.com/1395808 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Caroline Tice <cmtice@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [add] https://crrev.com/34b4506112b52a32844bcababd96a894f1cae0c4/PRESUBMIT.cfg
,
Jan 12
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/56620aa44e75db5710896fedcf53b4ea51606612 commit 56620aa44e75db5710896fedcf53b4ea51606612 Author: George Burgess IV <gbiv@google.com> Date: Thu Jan 17 20:18:22 2019 git-hooks: swap to `[[`; fix style nits Per the comment in I101c4c8b142f0c8f78819779adcfd0b3b0e1d614, `[[` is preferable to `test` where possible. Swap to that for consistency. (`[[` isn't an actual command, so we have to stick to `test` in status_to_tf) BUG= chromium:918755 TEST=check-style on a handful of Python files Change-Id: Ic2256fadc570a5d3818918e49de45d641a21574a Reviewed-on: https://chromium-review.googlesource.com/1415683 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/56620aa44e75db5710896fedcf53b4ea51606612/toolchain_utils_githooks/check-style [modify] https://crrev.com/56620aa44e75db5710896fedcf53b4ea51606612/toolchain_utils_githooks/check-format
,
Jan 18
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/toolchain-utils/+/ea973ec56fa436a801937111a6bd7218d1a1be1d commit ea973ec56fa436a801937111a6bd7218d1a1be1d Author: George Burgess IV <gbiv@google.com> Date: Fri Jan 18 12:01:37 2019 git-hooks: support linting outside of the source tree We appear to have workflows and committers that prefer to `push` from outside of the CrOS source tree. It's relatively straightforward to have fallbacks to support that, so I don't see why not. If you want to interact with this repository from outside of a CrOS source tree, you now have two options: - If you have a source tree somewhere on your machine, you can set CHROMEOS_ROOT_DIRECTORY to that path. If `check-lint` detects a usable `cros` in there, it'll function as usual. - If you don't have a source tree on your machine, `check-lint` will fall back to pylint-only linting (with a warning). This isn't ideal (we miss shell linting, as well as a few other nits in Python, somehow), but should get us most of the way to happiness. This also includes a refactor of `test` to `[[` BUG= chromium:918755 TEST=Ran check-lint on some python files in a checkout outside and inside of the source tree Change-Id: I101c4c8b142f0c8f78819779adcfd0b3b0e1d614 Reviewed-on: https://chromium-review.googlesource.com/1412682 Commit-Ready: George Burgess <gbiv@chromium.org> Tested-by: George Burgess <gbiv@chromium.org> Reviewed-by: Caroline Tice <cmtice@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/ea973ec56fa436a801937111a6bd7218d1a1be1d/toolchain_utils_githooks/check-lint |
||
►
Sign in to add a comment |
||
Comment 1 by g...@google.com
, Jan 3