{"thread":{"id":"51498","subject":"[PATCH 0/1] Travis CI: Lint for Python syntax errors and undefined names","startedAt":"2019-07-19T14:18:58Z","lastAt":"2019-07-20T12:51:34Z","messageCount":4,"participants":["Christian Clauss via GitGitGadget","cclauss via GitGitGadget","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"379098","messageId":"pull.290.git.gitgitgadget@gmail.com","threadId":"51498","inReplyTo":null,"subject":"[PATCH 0/1] Travis CI: Lint for Python syntax errors and undefined names","fromName":"Christian Clauss via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-19T14:18:54Z","receivedAt":"2019-07-19T14:18:58Z","isPatch":true,"sender":{"key":"name:Christian Clauss","avatar":null},"body":"Several things were changed between Python 2 and Python 3. Here we are\nmaking changes to make the code run on both Py2 and Py3. We are doing this\nbecause the end of life of Python 2 is in 167 days. We are using print() \nfunction because legacy print statements are syntax errors on Py3.reduce() \nwas moved in Python 3 and raw_input() was removed so we make changes to\navoid NameErrors at runtime. Signed-off-by: Christian Clauss cclauss@me.com\n[cclauss@me.com]\n\ncclauss (1):\n  Travis CI: Lint for Python syntax errors and undefined names\n\n .travis.yml                        |  4 +++\n contrib/fast-import/import-zips.py |  3 +-\n contrib/hg-to-git/hg-to-git.py     | 47 +++++++++++++++---------------\n git-p4.py                          |  3 ++\n 4 files changed, 33 insertions(+), 24 deletions(-)\n\n\nbase-commit: 9d418600f4d10dcbbfb0b5fdbc71d509e03ba719\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-290%2Fcclauss%2Fpatch-1-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-290/cclauss/patch-1-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/290\n-- \ngitgitgadget\n"},{"id":"379099","messageId":"ad1fdb86ae42378d10584deb58115adf11de8ef7.1563545933.git.gitgitgadget@gmail.com","threadId":"51498","inReplyTo":"pull.290.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] Travis CI: Lint for Python syntax errors and undefined names","fromName":"cclauss via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-07-19T14:18:55Z","receivedAt":"2019-07-19T14:19:01Z","isPatch":true,"sender":{"key":"name:cclauss","avatar":null},"body":"From: cclauss <cclauss@me.com>\n\nSeveral things were changed between Python 2 and Python 3.\nThere are a few Python 3 incompatibilities to work on.\nHere we are making changes to make the code run on both Py2 and Py3.\nWe are doing this because the end of life of Python 2 is in 167 days.\nWe are using print() function because legacy print statements are syntax\nerrors on Py3.\nreduce() was moved in Python 3 and raw_input() was removed so we make\nchanges to avoid NameErrors being raised at runtime.\nWe are also putting flake8 lint tests in place on Travis CI to avoid\nany backsliding on future pull requests.\n\nSigned-off-by: cclauss <cclauss@me.com>\n---\n .travis.yml                        |  4 +++\n contrib/fast-import/import-zips.py |  3 +-\n contrib/hg-to-git/hg-to-git.py     | 47 +++++++++++++++---------------\n git-p4.py                          |  3 ++\n 4 files changed, 33 insertions(+), 24 deletions(-)\n\ndiff --git a/.travis.yml b/.travis.yml\nindex ffb1bc46f2..62557dc8fd 100644\n--- a/.travis.yml\n+++ b/.travis.yml\n@@ -39,6 +39,10 @@ matrix:\n       compiler:\n       script: ci/test-documentation.sh\n       after_failure:\n+    - env: jobname=LintPython\n+      language: python\n+      before_install: pip install flake8\n+      script: flake8 . --count --select=E9,F63,F72,F82 --show-source --statistics\n \n before_install: ci/install-dependencies.sh\n script: ci/run-build-and-tests.sh\ndiff --git a/contrib/fast-import/import-zips.py b/contrib/fast-import/import-zips.py\nindex d12c296223..138194475c 100755\n--- a/contrib/fast-import/import-zips.py\n+++ b/contrib/fast-import/import-zips.py\n@@ -8,6 +8,7 @@\n ##  python import-zips.py *.zip\n ##  git log --stat import-zips\n \n+from __future__ import print_function\n from os import popen, path\n from sys import argv, exit, hexversion, stderr\n from time import mktime\n@@ -19,7 +20,7 @@\n     exit(1)\n \n if len(argv) < 2:\n-    print 'usage:', argv[0], '<zipfile>...'\n+    print('usage:', argv[0], '<zipfile>...')\n     exit(1)\n \n branch_ref = 'refs/heads/import-zips'\ndiff --git a/contrib/hg-to-git/hg-to-git.py b/contrib/hg-to-git/hg-to-git.py\nindex de3f81667e..1d5e5156a4 100755\n--- a/contrib/hg-to-git/hg-to-git.py\n+++ b/contrib/hg-to-git/hg-to-git.py\n@@ -18,6 +18,7 @@\n     along with this program; if not, see <http://www.gnu.org/licenses/>.\n \"\"\"\n \n+from __future__ import print_function\n import os, os.path, sys\n import tempfile, pickle, getopt\n import re\n@@ -42,7 +43,7 @@\n \n def usage():\n \n-        print \"\"\"\\\n+        print(\"\"\"\\\n %s: [OPTIONS] <hgprj>\n \n options:\n@@ -54,7 +55,7 @@ def usage():\n \n required:\n     hgprj:  name of the HG project to import (directory)\n-\"\"\" % sys.argv[0]\n+\"\"\" % sys.argv[0])\n \n #------------------------------------------------------------------------------\n \n@@ -104,22 +105,22 @@ def getgitenv(user, date):\n if state:\n     if os.path.exists(state):\n         if verbose:\n-            print 'State does exist, reading'\n+            print('State does exist, reading')\n         f = open(state, 'r')\n         hgvers = pickle.load(f)\n     else:\n-        print 'State does not exist, first run'\n+        print('State does not exist, first run')\n \n sock = os.popen('hg tip --template \"{rev}\"')\n tip = sock.read()\n if sock.close():\n     sys.exit(1)\n if verbose:\n-    print 'tip is', tip\n+    print('tip is', tip)\n \n # Calculate the branches\n if verbose:\n-    print 'analysing the branches...'\n+    print('analysing the branches...')\n hgchildren[\"0\"] = ()\n hgparents[\"0\"] = (None, None)\n hgbranch[\"0\"] = \"master\"\n@@ -155,7 +156,7 @@ def getgitenv(user, date):\n             hgbranch[str(cset)] = \"branch-\" + str(cset)\n \n if not hgvers.has_key(\"0\"):\n-    print 'creating repository'\n+    print('creating repository')\n     os.system('git init')\n \n # loop through every hg changeset\n@@ -180,27 +181,27 @@ def getgitenv(user, date):\n     os.write(fdcomment, csetcomment)\n     os.close(fdcomment)\n \n-    print '-----------------------------------------'\n-    print 'cset:', cset\n-    print 'branch:', hgbranch[str(cset)]\n-    print 'user:', user\n-    print 'date:', date\n-    print 'comment:', csetcomment\n+    print('-----------------------------------------')\n+    print('cset:', cset)\n+    print('branch:', hgbranch[str(cset)])\n+    print('user:', user)\n+    print('date:', date)\n+    print('comment:', csetcomment)\n     if parent:\n-\tprint 'parent:', parent\n+\t    print('parent:', parent)\n     if mparent:\n-        print 'mparent:', mparent\n+        print('mparent:', mparent)\n     if tag:\n-        print 'tag:', tag\n-    print '-----------------------------------------'\n+        print('tag:', tag)\n+    print('-----------------------------------------')\n \n     # checkout the parent if necessary\n     if cset != 0:\n         if hgbranch[str(cset)] == \"branch-\" + str(cset):\n-            print 'creating new branch', hgbranch[str(cset)]\n+            print('creating new branch', hgbranch[str(cset)])\n             os.system('git checkout -b %s %s' % (hgbranch[str(cset)], hgvers[parent]))\n         else:\n-            print 'checking out branch', hgbranch[str(cset)]\n+            print('checking out branch', hgbranch[str(cset)])\n             os.system('git checkout %s' % hgbranch[str(cset)])\n \n     # merge\n@@ -209,7 +210,7 @@ def getgitenv(user, date):\n             otherbranch = hgbranch[mparent]\n         else:\n             otherbranch = hgbranch[parent]\n-        print 'merging', otherbranch, 'into', hgbranch[str(cset)]\n+        print('merging', otherbranch, 'into', hgbranch[str(cset)])\n         os.system(getgitenv(user, date) + 'git merge --no-commit -s ours \"\" %s %s' % (hgbranch[str(cset)], otherbranch))\n \n     # remove everything except .git and .hg directories\n@@ -233,12 +234,12 @@ def getgitenv(user, date):\n \n     # delete branch if not used anymore...\n     if mparent and len(hgchildren[str(cset)]):\n-        print \"Deleting unused branch:\", otherbranch\n+        print(\"Deleting unused branch:\", otherbranch)\n         os.system('git branch -d %s' % otherbranch)\n \n     # retrieve and record the version\n     vvv = os.popen('git show --quiet --pretty=format:%H').read()\n-    print 'record', cset, '->', vvv\n+    print('record', cset, '->', vvv)\n     hgvers[str(cset)] = vvv\n \n if hgnewcsets >= opt_nrepack and opt_nrepack != -1:\n@@ -247,7 +248,7 @@ def getgitenv(user, date):\n # write the state for incrementals\n if state:\n     if verbose:\n-        print 'Writing state'\n+        print('Writing state')\n     f = open(state, 'w')\n     pickle.dump(hgvers, f)\n \ndiff --git a/git-p4.py b/git-p4.py\nindex 3991e7d1a7..9faee25db2 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -36,6 +36,8 @@\n     unicode = str\n     bytes = bytes\n     basestring = (str,bytes)\n+    raw_input = input\n+    from functools import reduce\n else:\n     # 'unicode' exists, must be Python 2\n     str = str\n@@ -3968,6 +3970,7 @@ def renameBranch(self, branch_name):\n                 break\n \n         if not found:\n+            sync = P4Sync()\n             sys.exit(\"gave up trying to rename existing branch {0}\".format(sync.branch))\n \n     def findLastP4Revision(self, starting_point):\n-- \ngitgitgadget\n"},{"id":"379114","messageId":"xmqqmuh9vmfa.fsf@gitster-ct.c.googlers.com","threadId":"51498","inReplyTo":"ad1fdb86ae42378d10584deb58115adf11de8ef7.1563545933.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] Travis CI: Lint for Python syntax errors and undefined names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-19T19:44:57Z","receivedAt":"2019-07-19T19:45:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"cclauss via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: cclauss <cclauss@me.com>\n\nI'll tweak this line (and your sign-off) to read as \"Christian\nClauss <cclauss@me.com>\" as you had in your cover letter.\n\n> Several things were changed between Python 2 and Python 3.\n> There are a few Python 3 incompatibilities to work on.\n> Here we are making changes to make the code run on both Py2 and Py3.\n> We are doing this because the end of life of Python 2 is in 167 days.\n\nAll sounds sensible, and the above is quite a good problem\ndescription.\n\n> We are using print() function because legacy print statements are syntax\n> errors on Py3.\n> reduce() was moved in Python 3 and raw_input() was removed so we make\n> changes to avoid NameErrors being raised at runtime.\n> We are also putting flake8 lint tests in place on Travis CI to avoid\n> any backsliding on future pull requests.\n\nNothing is wrong here, but the convention in our codebase is to\ndescribe the changes as if we are giving orders to the codebase \"to\nbe like so\".  And as you have enumeration, I would write something\nlike this if I were describing this change:\n\n\n     - Use the `print()` function, because Py3 no longer has the `print`\n       statement.\n\n     - Import `reduce()` from functools, because Py3 requires this, and\n       importing also works with Py2 (even though it wasn't necessary).\n\n     - Use `input()` instead of `raw_input()`, as the former can be used\n       with both but the latter was removed in Py3.\n\n    Also add a CI job to Travis CI to run flake8 lint to avoid an\n    backsliding on future pull requests.\n\nhttps://python-future.org/compatible_idioms.html#raw-input seems to\nsuggest, just like you import reduce from functools, you need to\nimport input from builtins.  Is it not the case?\n\n> +      script: flake8 . --count --select=E9,F63,F72,F82 --show-source --statistics\n\nHow was the set of \"select\"ed violations to catch chosen?  How are\nwe going to maintain this list going forward?\n\nThe rest of the patch looked sensible.\n\nThanks.\n"},{"id":"379134","messageId":"20190720125127.GK20404@szeder.dev","threadId":"51498","inReplyTo":"ad1fdb86ae42378d10584deb58115adf11de8ef7.1563545933.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] Travis CI: Lint for Python syntax errors and undefined names","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-07-20T12:51:27Z","receivedAt":"2019-07-20T12:51:34Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Jul 19, 2019 at 07:18:55AM -0700, cclauss via GitGitGadget wrote:\n> Several things were changed between Python 2 and Python 3.\n> There are a few Python 3 incompatibilities to work on.\n> Here we are making changes to make the code run on both Py2 and Py3.\n> We are doing this because the end of life of Python 2 is in 167 days.\n> We are using print() function because legacy print statements are syntax\n> errors on Py3.\n> reduce() was moved in Python 3 and raw_input() was removed so we make\n> changes to avoid NameErrors being raised at runtime.\n> We are also putting flake8 lint tests in place on Travis CI to avoid\n> any backsliding on future pull requests.\n\nIt seems to me that this patch does too many things at once, and\nperhaps it would be better to split it into a couple of smaller\npatches that do only one thing, e.g.:\n\n  - use print function instead of statement in 'hg-to-git' (which\n    constitutes the bulk of this patch),\n  - do the same in 'contrib/fast-import/import-zips.py'\n  - import 'reduce' and fix 'raw_input' in 'git-p4'\n  - and once all that is done and the linter runs clean, add the\n    linter job to Travis CI.\n\nThis would ease the job of the reader, now and in the future, and it\nwould better stand out that ...\n\n> diff --git a/git-p4.py b/git-p4.py\n> index 3991e7d1a7..9faee25db2 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n\n> @@ -3968,6 +3970,7 @@ def renameBranch(self, branch_name):\n>                  break\n>  \n>          if not found:\n> +            sync = P4Sync()\n\n... this change is not mentioned in the commit message.\n\nIs this something the linter complains about?\nIt doesn't look like a Python 2 vs. 3 compatibility fix to me, so it\nmight deserve a dedicated patch.\n\nCc-ing Luke for this bit.\n\n>              sys.exit(\"gave up trying to rename existing branch {0}\".format(sync.branch))\n>  \n>      def findLastP4Revision(self, starting_point):\n> -- \n> gitgitgadget\n"}]}