search

farmOS / farmOS.py

A Python library for interacting with farmOS over API.
GNU General Public License v3.0
27 stars 12 forks source link

GitHub Action to lint Python code #46

Closed cclauss closed 3 years ago

cclauss commented 3 years ago

Output: https://github.com/cclauss/farmOS.py/actions

paul121 commented 3 years ago

Thanks @cclauss !! This is great. I particularly like the codespell check!

I'd also like to add brief comments explaining what each of the steps/linters is doing unless it isn't super obvious.

Latest development is happening on the 1.x branch however so I might ask that you open a new PR against that branch. We already have a github action to run tests as well as run black linting but certainly welcome additions there: https://github.com/farmOS/farmOS.py/blob/1.x/.github/workflows/run-tests.yml#L43

On that note, I'm curious your thoughts/experience/best practice on whether linting should be added as a separate workflow, a separate job, or just additional step(s). I've seen other repos include a script that does all of the linting and simply call the script as part of a single step.

cclauss commented 3 years ago

Re-done against farmOS:1.x

When a GitHub Action fails, the contributor wants to quickly find exactly which command failed. This means that each test tool should have a separate run command. I do not hide the commands in a separate file because the indirection causes the contributor extra clicks and mental cycles each time a test fails.

paul121 commented 3 years ago

That's a good point. I want to review the tools a bit before merging but appreciate that you put this together!

paul121 commented 3 years ago

Made a couple changes to keep the testing and linting workflows separate. Merged! Thanks @cclauss.

cclauss commented 3 years ago

Nice work. It looks slick!