#123: Run a syntax checker in a pre-commit hook --------------------------+------------------------------------------------- Reporter: eschnett | Owner: mthomas Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+------------------------------------------------- Changes (by hinder):

* status: new => review

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.

#123: Run a syntax checker in a pre-commit hook --------------------------+------------------------------------------------- Reporter: eschnett | Owner: mthomas Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+-------------------------------------------------

Comment (by hinder):

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.

#123: Run a syntax checker in a pre-commit hook --------------------------+------------------------------------------------- Reporter: eschnett | Owner: mthomas Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+-------------------------------------------------

Comment (by eschnett):

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.