-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Migrate utilities specific to test/cmd/secrets.sh into the test script #11779
Migrate utilities specific to test/cmd/secrets.sh into the test script #11779
Conversation
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.
It looks like this change does two things:
(1) move files from various temporary directories under $BASETMPDIR
to $ARTIFACTS_DIR
(2) change quoting to be safer and to log variable values instead of names.
The commit message is terse, but the changes themselves look fine.
Utility functions that are one-off an not likely to be used in any other place by other scripts should live inline or next to the code that does use them. This commit migrates those functions to test/cmd/secrets.h as appropriate. Furthermore, we amend the location of temporary files that the functions create to correctly be placed in $ARTIFACT_DIR. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
c4f27c0
to
aa7f9bd
Compare
Squashed and added a real commit message. @danmcp this is ready to merge |
[merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11144/) (Image: devenv-rhel7_5319) |
Evaluated for origin merge up to aa7f9bd |
[Test]ing while waiting on the merge queue |
Evaluated for origin test up to aa7f9bd |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11144/) (Base Commit: f883216) |
@Miciah please review the second commit