-
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
Open
ianayl
wants to merge
14
commits into
sycl
Choose a base branch
from
ianayl/benchmark-ci-named-runs
base: sycl
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
f6d7cd3
Add sanitation and custom names to benchmark ci
ianayl 8ef6939
remove tab
ianayl 6e8ab8b
remove erroneous line
ianayl 17a97f8
fix typo
ianayl 6715088
update syntax
ianayl cfe18f9
fix typo
ianayl ed97724
add debug logging
ianayl 4e7abeb
add missing outputs section
ianayl 25b56e5
unfurl ternary to get logs
ianayl eeb1407
Allow capital letters in saved name
ianayl 50882b4
Disallow 1 character save names
ianayl 02fdea0
fix bug
ianayl aec29ab
Merge branch 'sycl' of https://github.com/intel/llvm into ianayl/benc…
ianayl 8e638e5
Update .github/workflows/sycl-ur-perf-benchmarking.yml
ianayl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
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.
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 ina-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