sncli icon indicating copy to clipboard operation
sncli copied to clipboard

Fix issue #129 pep-8 sorted imports

Open clach04 opened this issue 3 years ago • 3 comments

Used isort to automate. Now have per line imports following pep8 order. Remove duplicate datetime import.

clach04 avatar Nov 02 '22 02:11 clach04

Hmmm, easy answer is; your project, your rules ;-)

You might want to consider looking at:

  • black (very opinionated formatter)
  • isort (as I used in this instance, this is actually my first time using this in anger, I was very impressed with it)
  • pyflakes (or some other linting tool)

I don't have that many large projects so I've not invested much time into automation, ideas to check out:

  • manual make (or setup.py option)
  • git commit hook
    • https://towardsdatascience.com/keep-your-code-clean-using-black-pylint-git-hooks-pre-commit-baf6991f7376
    • https://ljvmiranda921.github.io/notebook/2018/06/21/precommits-using-black-and-flake8/
  • GitHub actions - e.g. https://black.readthedocs.io/en/stable/integrations/github_actions.html

Right now my advice would be don't rush into anything.

clach04 avatar Nov 03 '22 03:11 clach04

Yep that's fair, thanks. :+1:

Perhaps for now, a Makefile target that runs the isort command you used to generate these changes - then at least we have a record and can keep it consistent.

samuelallan72 avatar Nov 03 '22 07:11 samuelallan72

If this is still being considered..... I agree with using a workflow/pre-commit hook to automate formatting if you want to keep it standard. black is industry standard in terms of formatting but would probably change a lot in the codebase.

However, for now, instead of a Makefile you might want to add isort as a dependency and some docs on how to run it before you add any more automated or abstracted way to do it.

It would be best to keep this PR separate from any PR that implements automation. There isn't a lot of development going on so automation isn't an immediate need. Implementing it can be complex and would probably deserve it's own PR.

lostways avatar Jun 06 '23 20:06 lostways