Run a syntax checker in a pre-commit hook

Comments (8)

1. 

   Erik Schnetter reporter

   • marked as enhancement
   • removed comment

   • 2010-12-23T16:31:40+00:00
2. 

   Ian Hinder

   • changed status to open
   • removed comment

   The attached patch runs pylint and pychecker on every invocation of the "sim" shell script. On my laptop it adds approximately one second to the execution time for "sim help". It looks for the checker binary under several names. For example, fink installs pychecker as pychecker-py26.

   Currently, pychecker reports very few warnings, but pylint reports a large number. It would be good to understand and fix these, and if they are false positives, to add exceptions for them, before committing this patch.

   • 2011-03-08T07:19:20+00:00
3. 

   Erik Schnetter reporter

   • removed comment

   On my system, pylint is called "pylint-2.4".

   What do pychecker and pylint do? Do they only output warnings, or abort the code?

   In my experience, aborting on errors and on some warnings is the best thing to do. This may seem inconvenient at first, but very quickly one obtains a much more solid code base.

   • 2011-03-08T10:00:23+00:00
4. 

   Ian Hinder

   • removed comment

   They output warnings to the screen and return a nonzero exit code if there were any. I decided not to check the exit code in the current patch because that would stop people from running even for non-problems, and because there are currently a large number of reports from pylint, and a small number from pychecker.

   Once we have fixed all the current reports, or marked them as OK in the checkers, we can check the exit code and abort in the case of an error, but I think it's premature to do it until that is the case.

   I can add pylint-2.4 to the list of pylints. I don't know a good generic way of locating it.

   Michael: could you give the patch a try and see if you understand any of the warnings which are being produced? Some of them looked trivial to fix, such as missing docstrings, and we could even suppress that particular one for now. It would be good to get these down to zero.

   Of course, this should not take priority over the "blocker" and "critical" tickets.

   • 2011-03-08T10:36:08+00:00
5. 

   Erik Schnetter reporter

   • removed comment

   I updated the patch to include nicer output, in particular separating the checking output from the command output.

   All problems reported should be rather easy to correct. I suggest to apply the patch, in the hope to nag others into addressing some of the reported issues.

   • 2011-05-02T18:36:52+00:00
6. 

   Erik Schnetter reporter

   • removed comment

   I tried the patch for some time, and found that it didn't help. Most of the reported problems are trivial, while the additional delay is annoying. It also doesn't work on all systems; on some systems, an error message and the help text is output instead.

   I would now suggest to add a new simfactory command that runs the syntax checker, instead of running it all the time.

   • 2011-05-23T16:58:06+00:00
7. 

   Ian Hinder

   • changed status to resolved
   • removed comment

   I have converted the patch into a script simfactory/bin/sim-syntax-check - it is in r1418.

   • 2011-08-09T09:26:17+00:00
8. 

   Roland Haas

   • edited description
   • changed status to closed

   • 2019-02-21T20:17:43+00:00
9. Log in to comment