-
Couldn't load subscription status.
- Fork 793
[CI] Change pack_release -> release_toolchain_artifact input
#19646
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
Conversation
It's unused in trunk and is only passed in `sycl-rel-nightly.yml` in `sycl-rel-*` branches (trunk version of it is an empty dummy stub). Not sure how future "release" branches are expected to get one.
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.
Not sure how future "release" branches are expected to get one.
If I understand your point correctly - sycl-rel-(x+1) will take the sycl-rel-nightly.yml workflow file from the sycl-rel-x branch.
In general LGTM, just minor notes.
| type: string | ||
| required: false | ||
| description: | | ||
| If provided, create an addition toolchain artifact with the same name |
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.
Typo?
Same for windows
| If provided, create an addition toolchain artifact with the same name | |
| If provided, create an additional toolchain artifact with the same name |
| required: false | ||
| description: | | ||
| If provided, create an addition toolchain artifact with the same name | ||
| (`inputs.toolchain_artifact_filename`) as `toolchain_artifact` above |
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.
This is a bit confusing. It's an archive name, not an artifact name, isn't it? From this sentence it seems like it creates one more "sycl_linux_default" or so. Can't we say "If provided, create an additional toolchain artifact without utilities used for testing called inputs.release_toolchain_artifact or just "If provided, create an additional toolchain artifact without utilities used for testing"?
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.
@aelovikov-intel could you please also update it for win? Otherwise LGTM
It's unused in trunk and is only passed in
sycl-rel-nightly.ymlinsycl-rel-*branches (trunk version of it is an empty dummy stub). Not sure how future "release" branches are expected to get one.I think making the change results in better alignment between different artifacts produced the workflow, no other reasons beyond that.