CI cleanup

[MatthewSteen]

Close #194. Cleaned up the CI action by updating versions and reorganized into sequential jobs: style > tests > coverage. I held off on removing Pylama pending a discussion.

Pylama

I considered removing it but would like the team's feedback. In general I prefer "first-party" packages over third-party libraries. Currently, it looks like there's some overlap/redundancy in the packages we're using. For example, pylama wraps the following.

Organization	Package	Function
PyCQA	pycodestyle (formerly pep8)	code style
PyCQA	pydocstyle (formerly pep257)	docstring style
PyCQA	pyflakes	error checking
PyCQA	mccabe	code complexity
PyCQA	pylint	linting
Michele Lacchia	Radon	code metrics
Steven Myint	eradicate	removes commented code
Python	Mypy	static typing
Jendrik Seipp	Vulture	removes dead code

We're also running black, which does code styling and some docstring styling. Additionally, I'm not sure we need or want some of the tools above (e.g. mccabe) or if they work (e.g. vulture).

Proposal

I propose we remove Pylama, choose the ones from the table above we'd like to use directly, and add new ones as needed. Something like...

Style Job Commands (existing)

poetry run pylama
poetry run isort . --check
poetry run black . --check
poetry run mypy buildingmotif/*.py tests/*.py migrations/*.py

Style Job Commands (new)

poetry run pydocstyle
poetry run pylint
poetry run isort . --check
poetry run black . --check
poetry run mypy buildingmotif/*.py tests/*.py migrations/*.py

FYI @haneslinger, @gtfierro, @TShapinsky

[gtfierro]

Looks like flake8 can help wrap some of these (pyflakes, mccabe, pycodestyle), though it's not clear if pycodestyle would argue with black.

I like your "style job commands (new)" block but I'd like to include pyflakes and (possibly) mccabe

[TShapinsky]

Looks like flake8 can help wrap some of these (pyflakes, mccabe, pycodestyle), though it's not clear if pycodestyle would argue with black.

I like your "style job commands (new)" block but I'd like to include pyflakes and (possibly) mccabe

I'd be interested in exploring mccabe, it looks like it uses Cyclomatic complexity which could perhaps serve to increase traceability. I don't want to end up in a state where it is imposing unrealistic constraints (for the project) and we're just adding noqa everywhere.

As for this PR I think it looks good. My one critique is where may want to add names to more of the steps to make them easier to follow.

[gtfierro]

LGTM!

[MatthewSteen]

@gtfierro I replaced pylint with flake8 to simplify things. Looks like it passes now so will finish up by early next week. Let me know if you or @TShapinsky have any other comments.

[gtfierro]

Ran it on a couple of my other branches and it works great. I'm ok with you merging this once the conflicts are addressed

[gtfierro]

Looks like the docker-compose file and Makefile disappeared? I was using those! :)

I don't mind having a local Makefile that I use for development, but I wonder why the docker-compose file disappeared -- was that causing an issue for the CI?

[MatthewSteen]

Hmm, I didn't intentionally delete the Dockerfile but did the Makefile in favor or Poetry. Probably happened during my attempt to resolve conflicts.

[gtfierro]

I can definitely see how those might have gotten lost in a merge somewhere. If they weren't removed deliberately, then I can add then back in a future PR. This would be the Dockerfiles, docker-compose.yml and probably the .env file too

Is there a succinct way of formatting the code / testing using Poetry? Or do I just write out the poetry run pytest -s -vvvv ...

[MatthewSteen]

Looks like there were a lot of unintentional changes with my conflicts, sorry! I'll take a look through and try to clean it up in another PR.