-
Notifications
You must be signed in to change notification settings - Fork 79
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
Fix several issues in import_srpm #683
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Yann Dirson <[email protected]>
I was regularly losing time debugging this script exploding when just forgetting to use `-c`. Switching back to `master` after making changes and NOT committing them has just NO WAY to ever work. Signed-off-by: Yann Dirson <[email protected]>
import os | ||
import subprocess | ||
|
||
def call_process(args): |
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.
call
is probably clear enough in that context, and pleasantly shorter
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.
Hm, I'm a bit wary of using generic names, especially when it's so cheap to be more specific
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.
The internal wiki will need updating at the same time as this merge.
When we used `create_rpm_git_repo.py --local`, there may be no remote to pull from yet. Should indeed have been part of c2d89af. Signed-off-by: Yann Dirson <[email protected]>
Added another small fix still in the "fix import" scope, adjusting PR name to encompass it |
-c
No description provided.