-
Notifications
You must be signed in to change notification settings - Fork 38
Use ruff instead of pylint / Black #237
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
Conversation
Also removed py3.7 from test.yml so that the actions for this repo will use only py3.x which currently is resolving to 3.12 on the 3 platforms tested by the actions. If we'd prefer to keep a minimum supported version though we could add back py3.10 or whatever we want it to be. but py3.7 seemed to be unavailble and caused this actions failure: https://github.com/adafruit/cookiecutter-adafruit-circuitpython/actions/runs/10046678040/job/27766663798#step:3:7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good but I'm not sure how to test it. I can approve it if you've already tested it in some way.
I noticed a couple of things that should be updated, maybe not in this PR:
This is in the template, and is still 20.04:
Line 12 in 251be2c
os: ubuntu-20.04 |
This is the pre-commit for the repo (not the template pre-commit): It seems out of date: Ubuntu 20.04, and the actions versions may be old.
cookiecutter-adafruit-circuitpython/.github/workflows/pre-commit.yml
Lines 11 to 23 in 251be2c
jobs: | |
pre-commit: | |
runs-on: ubuntu-20.04 | |
steps: | |
- uses: actions/checkout@v1 | |
- uses: actions/setup-python@v2 | |
- name: set PY | |
run: echo >>$GITHUB_ENV "PY=$(python -c 'import hashlib, sys;print(hashlib.sha256(sys.version.encode()+sys.executable.encode()).hexdigest())')" | |
- uses: actions/cache@v1 | |
with: | |
path: ~/.cache/pre-commit | |
key: pre-commit|${{ env.PY }}|${{ hashFiles('.pre-commit-config.yaml') }} | |
- uses: pre-commit/action@v1.1.0 |
@dhalbert Thank you. I'll add a commit to change those to It can be tested locally by running cookie cutter and passing in the path to the local repo (set to this PR branch). i.e. assuming you have directories like this
you can run something like:
And it will run the version in your local instance at I have just figured out that it seems that our cookiecutter configuration doesn't support py3.12 though. I just tried running it this way and it ends up raising this exception (after asking the question prompts):
I'm pretty sure I recognize that pkg_resources thing as a 3.12 difference that came up in a different repo. |
...r.__dirname }}/{% if cookiecutter.sphinx_docs in ['y', 'yes'] %}.readthedocs.yaml{% endif %}
Show resolved
Hide resolved
In the latest commit I removed the pre_gen hook entirely. As far as I can tell this hook's intention is to enforce a minimum version of Looking at cookiecutter releases: https://pypi.org/project/cookiecutter/#history 1.7.3 is the version immediately prior to 2.1. I tried installing version 1.7.3 and running cookiecutter, this pre_gen hook doesn't get a chance to run and print the helpful error, instead some other part of cookiecutter raises an exception about a filename being too long. I also noticed that this hook runs after asking all the of the prompt questions. So even if it were functional it would have you answer all of the prompts before telling you that it won't be able to work. It'd be ideal to let the user know before the prompts if we know that it won't succeed. Modern version of cookiecutter do support a I tried adding the check to a few other spots but could not find a place to make it actually work. It would be nice if it were possible to specify it in cookiecutter.json but I have not found anything that suggests that is possible. Since it wasn't actually working, and it relies on something removed / refactored to a pypi module with python 3.12 I just removed it entirely in order to circumvent needing that extra module and having to manually install it in order for cookiecutter to run. |
Wierdly, there are no releases of this. Maybe we should at least label releases to keep track of changes and note incompatibilities, etc. |
I did figure out that you can specify a branch when you run the command with the
So if we did start making branches / tags with release versions it would be possible to do something like this to specify a version.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This changes the generated library to use ruff instead of pylint / Black.