msherry / flycheck-pycheckers

Multiple syntax checker for Python in Emacs, using Flycheck
GNU General Public License v3.0
63 stars 23 forks source link

Move default ignore codes from Python to lisp #5

Closed JulienPalard closed 6 years ago

JulienPalard commented 6 years ago

Fixes https://github.com/msherry/flycheck-pycheckers/issues/4

Rationale is: in pycheckers.py there is a nice "# Customization #" section but in flycheck-pycheckers it is not natural to edit pycheckers.py for customization, it's better to customize from lisp with a custom-set-variable.

So I moved the default ignore codes from the pychecker customization section to a flycheck-pycheckers variable.

/!\ that's the first time I modify a package, and I didn't find how to test it cleanly before pushing my changes. Any hint on how it's done would be greatly appreciated.

msherry commented 6 years ago

Thanks for this! At first glance it looks good to me. I'll test it soon and merge if it works out, but initial impression looks 👍.

To test it, you should just be able to evaluate the new value of flycheck-pycheckers-ignore-codes and verify that it gets passed properly to pycheckers.py. You'll have to make sure that the pycheckers.py is the one under test, not the one installed via MELPA, so to do that, I usually:

This is a bit cumbersome and could probably be automated more. Also, I'd love to add some tests for everything in the future to prevent regressions.

Thanks again for working on this!

msherry commented 6 years ago

Just tried it out, this seems to work great. Thanks for taking the time to implement this!

msherry commented 6 years ago

Oops, I spoke too soon -- it looks like this causes the MyPy type annotations to be wrong. It doesn't affect the working of the code, so I'll leave it merged and fix up the annotations separately.

msherry commented 6 years ago

Fixed in https://github.com/msherry/flycheck-pycheckers/commit/bb5f9168bdf998662174e360ef425bf4bb3897ca