-
Notifications
You must be signed in to change notification settings - Fork 797
[CI] Add ability to rename benchmarking CI run results #19734
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: sycl
Are you sure you want to change the base?
Conversation
…hmark-ci-named-runs
Test runs for all edge cases:
|
@intel/dpcpp-devops-reviewers May I get a review? Thanks! |
BENCHMARK_SAVE_NAME="$(echo "$SAVE_NAME" | tr -cd 'A-Za-z0-9_-')" | ||
fi; | ||
if [ -n "$COMMIT_HASH" ]; then | ||
commit_cleaned="$(echo "$COMMIT_HASH" | tr -cd 'a-z0-9')" |
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 does tr -cd 'a-z0-9'
do? What does it mean to clean the commit hash? - because few lines above we are anyway checking if commit hash is valid or not.
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.
-cd
is complement + delete: The command deletes all characters that isn't in a-z0-9
.
This is definitely redundant, but I recall PSE's having a policy against ever using raw input from the user directly, especially if it has a potential of being later used in a command. However, if I just use tr
, users wouldn't get an error if they inputted invalid input. So, the first few lines that check each input against a regex were mostly to notify the user incase they put in anything invalid.
I figured the time spent doing the extra regex check above the tr
command (and vice versa) would be negligible, thus went for this instead.
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.
TBH, I think we can just remove this. If user enters an invalid commit id or PR number, we'll just throw an error message and exit
Co-authored-by: Udit Kumar Agarwal <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.