Added a python helper for check_header.sh in git pre-commit script.

Review Request #1910 — Created March 13, 2015 and submitted

zundel
pants
1250
4f1ec21...
pants-reviews
benjyw, dturner-tw, jsirois, lahosken

On my box this speeds up the header check from 5 seconds to a little over 1 second.

 build-support/bin/check_header.sh        | 46 ++--------------------------------------------
 build-support/bin/check_header_helper.py | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 44 deletions(-)

~/Src/Pants zundel/cheader * git commit -a
Checking packages
Checking imports
Checking headers
ERROR: All .py files other than __init__.py should start with the following header:

# coding=utf-8
# Copyright YYYY Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                        unicode_literals, with_statement)


---
The following 1 file(s) do not conform:
  src/grody.py
JS
  1. LGTM, just small stuff.
    
    It does seem to make more sense to speed all the pre-commit checks up by having the script pass a list of the new and modified files to the check scripts instead and just have all check scripts operate on only the new/changed files.
    Presumably that's a much bigger time win and its pretty-much a win across all checks for ~free.
    1. The 'only check changed files' strategy assumes we will never want to change our checks, but I suppose it wouldn't be hard to invoke manually over all files in that very rare case where we do update the checks.

  2. It probably makes sense to respect user's PATHS:

    #!/usr/bin/env python2.7
    
  3. You might just make this a left-anchored regex string with 20\d\d embedded instead of YYYY and do a multiline match.  No doubt you were going for bash->py transliteration though for less change.
    1. Actually, I was just thinking that this would be faster than a regex, but I have no stats to back this up.

    2. I spent a small amount of time unsucessfully trying to get this working. The broken version took about 200ms instead of the 100ms I get with this version. It really isn't too slow, but I just lost interest.

  4. 2 blank lines between all the top-level functions.
  5. 
      
ZU
ZU
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks for the review John. Commit fa44550

Loading...