-
Notifications
You must be signed in to change notification settings - Fork 353
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
Break Arcade Build, Test, Validate SDK into stages #3530
Conversation
/azp run |
Pull request contains merge conflicts. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Looks great except for a few questions/nits.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
I'll merge this in the morning just so I'm around to respond if this causes pain for anyone taking an Arcade update. |
Does it make sense to wait until P9 in out? |
Sounds like a good plan to me. There's nothing motivating merging this sooner. |
I'm going to wait to merge this until after @JohnTortugo has merged his change and he and @mmitche have clean builds. |
…)" This reverts commit b764063.
PR validation build - https://dev.azure.com/dnceng/public/_build/results?buildId=308614&view=results
Internal validation build - https://dev.azure.com/dnceng/internal/_build/results?buildId=308617&view=results
A couple of things to note:
@sunandabalu, @JohnTortugo, I changed how the post-build "dependsOn" stuff was evaluated. It appeared that the dependsOn was being passed either as a string, or with an array syntax. ie,
dependsOn: build
ordependsOn: [build]
. The first syntax doesn't permit you to specify multiple dependent jobs / stages. The second syntax looks nice, but (in my testing) seemed to only evaluate one of the dependent jobs, not all of them. I only call this out as an awareness thing when writing yaml.it appears that dependsOn is case-sensitive so I continued with the current naming pattern (pascal cased)
This change decreases official build time by 10+ minutes because we can run the "validate sdk" stage in parallel with the "post-build validate" stage instead of serializing the stages. I don't notice any significant impact to PR times (positive or negative) with moving artifacts between stages.
This change uses hosted pools for the Test stage, so we free up BYOC machines in the PR builds about 10 minutes earlier.
Removing "helix" telemetry, but leaving the "enableTelemetry" option because of Set the DOTNET_CLI_TELEMETRY_PROFILE environment variable #2688
Deprecating the "enablePublishBuildArtifacts" "enablePublishBuildAssets" parameters with an "artifacts" parameter which permits more control over what / how artifacts are published / downloaded.
The "artifacts" parameter is our supported way for moving artifacts between stages (for Arcade builds). It would be great if the "PackageArtifacts" stuff used the same artifacts rather than doing an additional upload - @JohnTortugo, I'll file an issue about this.
@alexperovich , changed
_BuildConfig
toConfiguration
inUnitTests.proj
because_BuildConfig
is supposed to be a yaml only concept and is intended to not interfere with any MSBuild or other build-time code.