Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Re-enables auto benchmarking / acceptance testing #78
Re-enables auto benchmarking / acceptance testing #78
Changes from 25 commits
32c1e29
6312791
937be91
5c6ebfa
ca1ce8e
df58314
1adc5f9
f2fab6a
dca29b0
46f0463
6e366f1
8937c46
183e41a
47091dd
738a6c2
71d1a2b
c68feb2
dd6b523
a5fe09b
5bc5cad
92bc042
bd8cf40
037545b
3c4a84a
6990c5a
87984e5
2256778
d54638f
38a70a5
b673876
632b196
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can you use the
env
feature of the GH workflow forBENCHMARK_API_KEY_PROD
andSLACK_URL_DEV_SCIENCE
?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.
What do you mean? 🤔
This is necessary to actually surface the secret as an environment variable in the bash sense.
env
of Github is used to pass environment variables between steps and/or jobs.I don't think you need to worry about the secrets leaking this way. As far as I can tell this is the intended way of things to work. Github removes the secrets from the log output (see image).
I can rewrite the script to accept these as arguments via argparse (bypassing environment variables), but the call to it would still be logged like this, I think:
cc @dirkschumacher
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.
No my comment just referred to using the yaml
env
key within a step instead ofexport
. I thought that would be the same. But no strong opinion, export obviously also workse.g.
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.
Sure, works for me. Does indeed look a little nicer. 😊
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.
Could have used
nextmv.Options
for this 👻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.
Haha, fair. I suppose I should get used to it more. 😊