Skip to content

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

Merged
merged 5 commits into from
Jul 24, 2024
Merged

Conversation

FoamyGuy
Copy link
Contributor

This changes the generated library to use ruff instead of pylint / Black.

@FoamyGuy
Copy link
Contributor Author

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

@FoamyGuy FoamyGuy requested a review from a team July 22, 2024 18:59
Copy link
Contributor

@dhalbert dhalbert left a 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:

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.

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

@FoamyGuy
Copy link
Contributor Author

@dhalbert Thank you. I'll add a commit to change those to ubuntu-latest

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

/somedir/cookiecutter-adafruit-circuitpython/
/somedir/cookiecutter_workspace/

you can run something like:

cd /somedir/cookiecutter_workspace/
cookiecutter ../cookiecutter-adafruit-circuitpython/

And it will run the version in your local instance at /somedir/cookiecutter-adafruit-circuitpython/

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):

/tmp/tmppnjckp3t.py:6: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  from pkg_resources import packaging
Traceback (most recent call last):
  File "/tmp/tmppnjckp3t.py", line 6, in <module>
    from pkg_resources import packaging
ImportError: cannot import name 'packaging' from 'pkg_resources' (/home/timc/repos/circuitpython/.cpy_venv/lib/python3.12/site-packages/pkg_resources/__init__.py)
ERROR: Stopping generation because pre_gen_project hook script didn't exit successfully
Hook script failed (exit status: 1)

I'm pretty sure I recognize that pkg_resources thing as a 3.12 difference that came up in a different repo.

@FoamyGuy
Copy link
Contributor Author

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 cookiecutter but it doesn't seem to actual functionally do that.

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 pre_prompt hook which theoretically could be used for that, but it was added more recently than 2.1 so any version of cookiecutter that supports pre_prompt we know will already support our cookie template.

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.

@dhalbert
Copy link
Contributor

Wierdly, there are no releases of this. Maybe we should at least label releases to keep track of changes and note incompatibilities, etc.

@FoamyGuy
Copy link
Contributor Author

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 --checkout CLI argument i.e.

 cookiecutter gh:foamyguy/cookiecutter-adafruit-circuitpython --checkout use_ruff

So if we did start making branches / tags with release versions it would be possible to do something like this to specify a version.

 cookiecutter gh:adafruit/cookiecutter-adafruit-circuitpython --checkout 1.1.0

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dhalbert dhalbert merged commit df3d4dd into adafruit:main Jul 24, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants