chore: extract glob for pyupgrade to separate script for cross-platform compatibility#950
chore: extract glob for pyupgrade to separate script for cross-platform compatibility#950peschuster wants to merge 1 commit intoCycloneDX:mainfrom
Conversation
tools/run_pyupgrade.py
Outdated
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from pyupgrade._main import main |
There was a problem hiding this comment.
the pyupgrade._main is internal of a 3rd party package.
we dont want to call it like that.
call the CLI programmatically, like so - but maek sure that the args are properly escaped if needed ....
subprocess.run(
[sys.executable, "-m", "pyupgrade", ...],
)or use runpy.run_module or something.
tools/run_pyupgrade.py
Outdated
| str(p) | ||
| for d in ['cyclonedx', 'typings', 'tests', 'tools', 'examples'] | ||
| for ext in ['*.py', '*.pyi'] | ||
| for p in Path(d).rglob(ext) |
There was a problem hiding this comment.
these d shall be paths relative to the repository root.
current behavior searches them relative to the current working dir, right?
could this be changed so that Path(d) is looked up relative to repo root?
There was a problem hiding this comment.
I gave this some thought and would change it a little bit different: keep the list of paths in tox.ini and use the python script just as a wrapper to perform the glob. The responsibility for passing the correct paths (relative to the current working directory or absolute) would stay at the caller and the script would not have to need any knowledge about the repository. Would you agree with that approach?
0730b36 to
f91065d
Compare
…rm compatibility 'sh' in tox.ini does not work on Windows in PowerShell. Signed-off-by: Peter Schuster <p.schuster@pilz.de>
f91065d to
53bb407
Compare
Currently pyupgrade cannot be run on Windows due to 'sh' in tox.ini not working in PowerShell.
Adding a separate script for this might be controversial.
I could not find another solution that is platform independent, except from inline python in tox.ini which got "complicated" due to
{posargs}. However, if anyone has a better idea, this could be reworked.AI Tool Disclosure
Affirmation