Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding isort and black #3

Open
jonasrk opened this issue Feb 3, 2020 · 1 comment
Open

Adding isort and black #3

jonasrk opened this issue Feb 3, 2020 · 1 comment

Comments

@jonasrk
Copy link

jonasrk commented Feb 3, 2020

Thanks, this is a real game changer for me! :)

If I were to customize it for myself, I would add auto-formatting with black and auto-sorting-imports with isort.

Potentially, I would also add https://github.com/python-poetry/poetry or make it obvious somewhere that it should be used.

Oh, and I think some tools (e.g. flake8, pylint) should also appear in requirements.txt ?

Happy to open a PR if you like any of those ideas. :)

@MartinHeinz
Copy link
Owner

First of all, thanks for suggestions and sorry for late response.

Regarding these suggested additions:

  • Black - Black is good tool if you have hard time being consistent with your formatting or are working in team where everybody has different style, as it forces its opinionated styling. I personally am pretty consistent with formatting and therefore I prefer to not add additional (unnecessary) tool.

  • isort - Honestly, I haven't heard of it before and I like. That said I'm pretty sure pylint/pep8 will warn you if you have wrong order of imports (library vs. internal).

  • Poetry - I'm not too familiar with the tool so excuse my ignorance. When it comes to dependency management, i don't see how it would be better than existing solution here. As for the building, packaging, publishing - it seems very nice for the cases when you are building Python packages (whl) and publishing to PyPi. That said, this blueprint uses mainly Docker images and I don't see how it would be beneficial to use Poetry here. Venv does the job just fine.

About the requirements.txt - flake8, pylint... are not in requirements.txt because the application is not dependent on these tools. If they were added to requirements.txt, that would mean that they would be also part of final image which doesn't really make sense. These tools are specifically downloaded in GitHub Actions Jobs, when the checks are ran.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants