-
Notifications
You must be signed in to change notification settings - Fork 206
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
Add official build support to standalone templates #261
Conversation
072fdb7
to
0fca56f
Compare
publishingInfraVersion: 3 | ||
validateDependsOn: | ||
- build | ||
enableSourceLinkValidation: true |
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.
should we also enable validation/publishing of symbols or does that happen automatically?
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.
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.
@safern yes, I believe that's still the case.
parameters: | ||
displayName: ${{ format('{0} {1}', parameters.osGroup, parameters.archType) }} | ||
name: ${{ format('{0}_{1}', parameters.osGroup, parameters.archType) }} | ||
enableTelemetry: ${{ parameters.isOfficialBuild }} |
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 must be missing it but where were we passing isOfficialBuild before? Looks like we were just not passing it and always defaulting to empty here right?
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'm passing it down here: https://github.com/dotnet/runtimelab/pull/261/files#diff-d71a1a613ea5de9e905a5c6f641d8c4c79800b54c4667cbcbefdfb2f91a898ddR57
and then the default of the parameter is set to false: https://github.com/dotnet/runtimelab/pull/261/files#diff-6a89a2c86af1832dfc215647fe7dd379b4def9cea440cf14ef179b65eb55e94aR7
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.
Yeah, I saw that. I meant before this change, we seem to have been using it to set the enableTelemetry field but we weren't actually setting it on the main yml or importing it unless I'm missing something.
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.
Ah I see what you're saying. The arcade templates ignore true if it is in an official build.
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.
LGTM thanks @safern!
For easier review I recommend disabling white space diffs.
I tested with: https://dev.azure.com/dnceng/internal/_build/results?buildId=862898&view=logs&j=226748d0-f812-5437-d3f0-2dd291f5666e
FYI: @AaronRobinsonMSFT @eerhardt @pgovind