migrate to QuickJS and prepare for 0.6.0 #85
Conversation
webknjaz
left a comment
webknjaz
left a comment
There was a problem hiding this comment.
I recommend separating ruff format into a dedicated commit/PR and then recording it in blame ignore — it's supported natively by GH: https://github.com/cherrypy/cheroot/blob/662cd9dcf426253536f921c618b480e65a429cb1/.git-blame-ignore-revs / https://github.com/ansible/awx-plugins/blob/274b32a7ab9eb09493b05e28eb5cad7441ec08bf/.git-blame-ignore-revs
| @@ -0,0 +1,45 @@ | |||
| [build-system] | |||
| requires = ["setuptools>=64", "wheel"] | |||
There was a problem hiding this comment.
Wheel was never needed here
| requires = ["setuptools>=64", "wheel"] | |
| requires = ["setuptools>=64"] |
| "Programming Language :: Python :: 3.9", | ||
| "Programming Language :: JavaScript", | ||
| ] | ||
| dependencies = ["argparse; python_version == '2.6'"] |
There was a problem hiding this comment.
This is rather pointless given the classifiers
| dependencies = ["argparse; python_version == '2.6'"] |
| "Programming Language :: Python :: 3.12", | ||
| "Programming Language :: Python :: 3.11", | ||
| "Programming Language :: Python :: 3.10", | ||
| "Programming Language :: Python :: 3.9", |
There was a problem hiding this comment.
Specify requires-python so that the depresolver takes it into account.
|
|
||
|
|
||
| DukPy is a simple javascript interpreter for Python built on top of | ||
| duktape engine **without any external dependency**. |
There was a problem hiding this comment.
It'd be nice to leave a historic reference explaining the project name.
There was a problem hiding this comment.
Consider integrating https://pypi.org/p/vendoring. It's Pradyun's thing used in pip.
| @@ -0,0 +1,7 @@ | |||
| line-length = 88 | |||
| target-version = "py39" | |||
There was a problem hiding this comment.
Set requires-python in pyproject.toml instead.
| target-version = "py39" |
There was a problem hiding this comment.
I recommend annotating whatever rules you list: https://github.com/cherrypy/cheroot/blob/662cd9dcf426253536f921c618b480e65a429cb1/.ruff.toml
There was a problem hiding this comment.
Use native integration, not local and you won't need to install ruff externally + run it in https://pre-commit.ci. Pro tip: with a double run, you can normalize trailing commas nicely — https://github.com/cherrypy/cheroot/blob/662cd9dcf426253536f921c618b480e65a429cb1/.pre-commit-config.yaml#L62-L87
| - uses: actions/checkout@v2 | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v2 |
There was a problem hiding this comment.
These versions are quite outdated.
| pip install ruff pre-commit | ||
| - name: Ruff check | ||
| run: | | ||
| ruff check . | ||
| ruff format --check . |
There was a problem hiding this comment.
Don't install ruff, run pre-commit (although, you could also skip GHA and use the official app). Personally, I wrap linting things with tox: https://github.com/cherrypy/cheroot/blob/662cd9dcf426253536f921c618b480e65a429cb1/.github/workflows/ci-cd.yml#L606-L710
Thanks for the review @webknjaz ! I really appreciate it. |
|
Yeah, I figured. Just wanted to post some feedback regarding easily fixable stuff. |
2 participants
No description provided.