[PATCH] D80978: Add a git hook script that can be manually setup to run some checks on every push

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 18 21:48:02 PDT 2020


MaskRay added inline comments.


================
Comment at: llvm/utils/git/pre-push.py:137
+
+def program_exists(cmd):
+    if sys.platform == 'win32' and not cmd.endswith('.exe'):
----------------
mehdi_amini wrote:
> mehdi_amini wrote:
> > MaskRay wrote:
> > > shutil.which
> > Does it handle the `.exe` suffix addition here?
> Seems like it would work, but requires 3.3, not sure if we should require this?
> 
> 
Python 3.4 was released 6 years ago. I think we don't need to care about 3.3 on Windows.


================
Comment at: llvm/utils/git/pre-push.py:40
+
+try:
+    dict.iteritems
----------------
Not used in Python 3


================
Comment at: llvm/utils/git/pre-push.py:49
+dev_null_fd = None
+z40="0000000000000000000000000000000000000000"
+
----------------
space around `=`


================
Comment at: llvm/utils/git/pre-push.py:76
+        query = input('%s (y/N): ' % (prompt))
+        if query.lower() not in ['y','n', '']:
+           print('Expect y or n!')
----------------
space after comma


================
Comment at: llvm/utils/git/pre-push.py:122
+            for l in stdout.splitlines():
+                log_verbose("STDOUT: %s" % l)
+        return stdout
----------------
Quotes are very inconsistent in this file. Please stick with `'`


================
Comment at: llvm/utils/git/pre-push.py:184
+    for sha in revs:
+      msg = git("log","--format=%B","-n1", sha)
+      if "Differential Revision" not in msg:
----------------
Single quotes and add spaces after commas


================
Comment at: llvm/utils/git/pre-push.py:224
+      handle_push(args, local_ref, local_sha, remote_ref, remote_sha)
+
----------------
No trailing empty line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80978/new/

https://reviews.llvm.org/D80978





More information about the llvm-commits mailing list