-
Notifications
You must be signed in to change notification settings - Fork 26
PART 2: Rewrite bash scripts to python - After #498+#505 #499
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
base: master
Are you sure you want to change the base?
Conversation
4289b5f to
4629b34
Compare
Also adding a "RUNNER" make variable that defaults to "uv run" but can also be overwritten, e.g. to just an empty string to use the current Python environment.
Verified manually with ``` python3 src/os_autoinst_scripts/chat_notify.py matrix.org "test message from okurz" "$token" "$room_id" ``` which sent a message to a matrix room.
4629b34 to
513f245
Compare
I would prefer one or multiple more descriptive namespaces, rather than making it even more cryptic. |
meaning what, to keep "src/os_autoinst_scripts/"? |
Martchus
left a comment
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.
Looks generally good so far.
After
One main open point is where we should place the Python files. It would be good to have them in a sub-directory and a usual Python practice is to use a properly named package derived from the directory name so "src/os_autoinst_scripts/" sounds correct but is also rather long and also means for testing we need to adjust the Python import path. How about just "oas/" for (Os-Autoinst-Scripts) and ditch the "src/"?