Repository navigation
Linting Requirements on Travis-CI #770
Description
So I was mucking around the depths of Travis-CI and realized that it supports multi-stage builds/testing. As it turns out, we're already doing this, as pushing to PyPI has been added as a "deploy" stage.
The other common use for multi-stages is to have a "linting" stage, to confirm code style before running the main body of tests. I wrote a Pull Request (#769) to add black as such a linting test, but maybe it would be good to hear others' thoughts on this.
-
Should we have a mandatory linting stage, required to pass before a Pull Request is generally accepted? On the plus side, this keeps the code clean, and hopefully makes it easier to people to jump right into the code base. On the minus side, this can sometimes serve as an impediment to (new) people submitting pull requests.
-
If we have this linting stage, is there any other linters that we should include?
blackis focused on coding style. Thoughts that come to mind include:- isort -- sorts imports -- I like this a lot, but it does take a little configuration to get black and isort to play nice together.
- mypy -- I don't think we currently use type hints, and it may not make sense on a codebase of this size.
- flake8 -- code issues and "smells" -- under the hood it seems to call
pycodestyle,pyflakes, andmccabe - is the poetry lockfile up to date -- see Fail Travis CI if poetry lockfile is not up to date #747 -- Personally, I think this is a helpful check, but it's tied to a particular date and time in a way that other linters aren't, meaning it could pass now, but not in an hour (if an update to a dependency is released)
- some others for consideration --> https://pre-commit.com/hooks.html
As a side, it would be nice if the fix were simple for a bot to come along after a failing test and propose the code that would fix it as a Pull Request to the Pull Request.
Activity
[-][Discussion] Linting Requirements on Travis-CI[/-][+]Linting Requirements on Travis-CI[/+]micahellison commentedon Dec 11, 2019
The mandatory linting sounded a little draconian to me at first, but after some consideration, I think it's preferable to just having a warning. The alternative would be that an accepted PR that doesn't pass linting could keep subsequent PRs in a warning state, through no fault of their contributors.
minchinweb commentedon Dec 12, 2019
I feel like black isn't too bad in this case either, because you can run
blackover your code and it will fix it for you.(jrnl-org#770) run black formatter on codebase for standardization
Merge pull request #778 from wren/black-format-770
stale commentedon Feb 10, 2020
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
minchinweb commentedon Feb 10, 2020
Do we want to add
isortto this list?stale commentedon Apr 11, 2020
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
[Travis] add linting via pyflakes
[Travis] add isort formatter
(#770) run black formatter on codebase for standardization