-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Remove pr: none clause from several CoreCLR pipelines #42933
Conversation
It looks like you missed runtime-coreclr outerloop? (just fixed that in AzDO) |
@@ -24,9 +24,6 @@ trigger: | |||
- eng/pipelines/libraries/* | |||
- eng/pipelines/runtime.yml | |||
|
|||
|
|||
pr: none |
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.
Do we ever want this pipeline to be triggerable?
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.
Or is that the outerloop pipeline?
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.
Yes, ci.yml corresponds to the "runtime-coreclr outerloop" pipeline. I think we should rename the YAML script to "outerloop.yml", this is an endless cause of confusion.
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.
I proposed to rename it before as well. However, if we rename it now, we will need to port that to release/5.0 and release/5.0-rc2 branch.
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.
Maybe it's just busywork. I would say let's leave this for a more suitable time when there are no porting concerns.
Hmm would be great if you could amend your commit or do something so that CI is retriggered. |
Good, I think the outerloop is no longer automatically triggered upon the PR, I fixed the pipeline parameters in the AzDO UI editor. |
/azp:run runtime-coreclr crossgen2 |
/azp run runtime-coreclr crossgen2 |
Azure Pipelines successfully started running 1 pipeline(s). |
@ViktorHofer - any additional feedback on this change you asked me to make? Or is there some remaining feedback from you I haven't yet addressed? |
Hmm wasn't there an issue that was tracking this work? |
Thanks Tomas! |
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.
Thanks!
I have updated the AzDO UI properties per Viktor's advice to enable these runs for
/asp run
runs. After verifying they work as expected we can expand the pipeline set. For now I have skipped the various stress runs that would be the most detrimental if they unexpectedly started to run as part of PR's as an unforeseen effect of the changes.Thanks
Tomas
/cc @dotnet/runtime-infrastructure