cylc-flow icon indicating copy to clipboard operation
cylc-flow copied to clipboard

Published 20 hours ago • verified publisher icon cylc

Add pre commit configuration (take 2)

Open hjoliver opened this issue 4 years ago • 4 comments

Supersede #3467

These changes partially address cylc/cylc-admin#64

This is @sgaist's branch rebased onto master, with some superficial style changes reverted in setup.py for a minimal diff with current master (so I can investigate why pycodestyle is failing on the CI platform on that branch).

Requirements check-list

  • [x] I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • [x] Contains logically grouped changes (else tidy your branch by rebase).
  • [x] Does not contain off-topic changes (use other PRs for other changes).
  • [x] Does not need tests (coding style validation only).
  • [x] No change log entry required (invisible to users).
  • [x] No documentation update required.
  • [ ] Created an issue at cylc-flow conda-forge repository with version changes (if you changed dependencies in setup.py, see recipe/meta.yaml).
  • [ ] No dependency changes.

hjoliver avatar Dec 15 '20 22:12 hjoliver

(same pycodestyle problem here ... maybe it is flake8 failing - it runs pycodestyle, but the new flake8 config overrides tox.ini??)

hjoliver avatar Dec 15 '20 23:12 hjoliver

Ah, that must be it. Added the old tox.ini warnings to the new flake8 config, and the tests are passing.

hjoliver avatar Dec 15 '20 23:12 hjoliver

Last commit removes pycodestyle from the Actions workflow, since flake8 runs pycodestyle anyway.

hjoliver avatar Dec 15 '20 23:12 hjoliver

@hjoliver I think this will produce pull requests with a lot more changes than what the OP intended to. Should we tweak the settings a bit to be more lenient and reduce the changes with each PR, or should we use as-is, to slowly improve the code base?

Good question. I modified and staged scheduler.py to see what pre-commit run would do. Seems to me all the changes are good ones, but yes it would create confusing PRs for a while.

We could consider merging this, then processing everything with flake8 etc. in one PR (or a few), to reset the bar, then carry on with consistent style from that point on.

hjoliver avatar Dec 16 '20 00:12 hjoliver

(I'm closing this for now as we haven't managed to get it up the priority list, and it's likely to a while yet ... can possibly revisit at a later date).

hjoliver avatar Sep 21 '22 20:09 hjoliver