-
Notifications
You must be signed in to change notification settings - Fork 403
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
Support repo change between invocations #499
Support repo change between invocations #499
Conversation
|
Welcome @sed-i! |
Replaces the hard-coded literal "origin" with the --repo value. This way cleanup takes place even if --repo value changes between invocations.
62fa46b
to
50f6327
Compare
/assign @thockin |
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.
Thanks! But no good deed goes unpunished.
I try not to use the global flag vars outside of main() - can you see about passing the repo around as an argument, so we can use it in these places? In the v4 (dev) branch, I made a gitRepo type to carry the repo, branch, etc. Not asking you to port all that, but just so you get the point.
Also, can you add a test to test_e2e.sh?
And finally, if you feel industrious, you can also port to v4 (master) branch. Or I can do that if you prefer.
I added a test, but it fails with
When I re-ran manually the test steps, it worked without error. Any idea what could be the reason? |
... missing a volume mount, that's why!
Will fix shortly. |
Sure, porting to v4 seems straightforward. I can submit a PR against master after we're happy with this one. |
@thockin Is this ok now? |
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.
Thanks!
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sed-i, thockin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Following the thick hint offered by @thockin in #497, this PR replaces the hard-coded literal
"origin"
with the--repo
value.This way cleanup takes place even if
--repo
value changes between invocations.Fixes #497