-
Notifications
You must be signed in to change notification settings - Fork 45
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
WIP: Python language support #423
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #423 +/- ##
==========================================
- Coverage 82.46% 82.24% -0.23%
==========================================
Files 42 42
Lines 13569 13652 +83
==========================================
+ Hits 11190 11228 +38
- Misses 2379 2424 +45
Continue to review full report at Codecov.
|
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.
👍
pym/bob/languages.py
Outdated
Resolver = PythonResolver | ||
|
||
HELPERS = dedent("""\ | ||
from subprocess import run, call, check_call, check_output |
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.
Not sure if I'm fully understanding what's going on here, but it looks like these are inserted into the final script. If this is the case: do you really want to clutter every user-provided script with those functions? Especially run
and call
are pretty generic and might be used by the user. I'd prefer import subprocess
like the ones below, even if that means more work here.
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.
Yeah, that's something where I'm not yet sure about. At least some minimal imports are required by the script prologue (os, os.path, sys). Almost any script will use the subprocess main functions anyway. I first found it convenient to implicitly import them always. But thinking more about this it might not be a good idea and leave this entirely to the user. That can be handled by some class(es) and {checkout,build,package}SetupPython
snippets.
Extracting included files in the executed script itself requires some code for each supported language. It's also cumbersome to clean up. Instead let the invoker handle the dirty details and just add the right reference in the final script.
This is the first draft to support Python as scripting language. Some parts are still missing and it's almost untested...