search

ywangd / stash

StaSh - Shell for Pythonista
MIT License
1.92k stars 227 forks source link

Lint with ruff on supported versions of CPython #474

Closed cclauss closed 1 year ago

cclauss commented 1 year ago

Ruff supports over 500 lint rules including bandit, flake8, isort, pylint, and pyupgrade and is written in Rust for speed.

cclauss commented 1 year ago

@mkb79 @bennr01 Your reviews, please.

bennr01 commented 1 year ago

Looks mostly good and runs with no errors.

Two minor problems:

  1. pull-requests are supposed to go the the dev branch
  2. If you want to replace flake8 with ruff, you should also update setup.py and add it to the TEST_REQUIREMENTS, so we can install all dev requirements with a quick pip install .[testing].

Also, should we really replace flake8 with a non-python software? This could complicate development on pythonista...

cclauss commented 1 year ago
  1. Switched to dev branch. I am not a fan of contributions going to a non-default branch. It is unintuitive. When I look at a repo, I expect to see the current state of the code. If dev is where contributions should go then dev should be the default branch. There can be a master/main which is a non-default branch.
  2. ruff will not install with pip from Python 2.7 or 3.6 so we should not move it into .[testing] until we stop supporting legacy versions of Python.

The JavaScript community has benefited mightily from having its build tools rewritten in Rust. I believe that the Python community should not miss out on the opportunity for improved development velocity.

bennr01 commented 1 year ago

I am not a fan of contributions going to a non-default branch. It is unintuitive. If dev is where contributions should go then dev should be the default branch. There can be a master/main which is a non-default branch.

An understandable complaint, keeping track of the correct branches for each repo can be very annoying. But there's a good reason for using this branch layout. The approach is called something like git flow if I recall correctly. The idea is that the default, master branch should always contain a usable version (no untested changes, no features that are currently in developement, ...) while all developments are made in individual feature branches and collected in the development branch. This way, it is guaranteed that the main/master/whatever branch always contains usable software and a user does not need to figure out which commit has working features.

The JavaScript community has benefited mightily from having its build tools rewritten in Rust. I believe that the Python community should not miss out on the opportunity for improved development velocity.

There may be a misunderstanding here: I am not denying it's usefulness. Rather, I am concerned that it won't run in the environment we are primarily targeting (the pythonista app), which may complicate development of StaSh while using said python environment.

ruff will not install with pip from Python 2.7 or 3.6 so we should not move it into .[testing] until we stop supporting legacy versions of Python.

Ok, that's a good reason. Also, I've started deprecating py2 in #475 for the app.

cclauss commented 1 year ago

I know git flow but as far as I understand, it does not mandate that master / main is the default branch of a repo.