-
Notifications
You must be signed in to change notification settings - Fork 5.3k
NO-MERGE: use staging Helix #123846
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
base: main
Are you sure you want to change the base?
NO-MERGE: use staging Helix #123846
Conversation
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.
Copilot wasn't able to review any files in this pull request.
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.
Copilot wasn't able to review any files in this pull request.
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.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
| WaitForWorkItemCompletion: true # optional -- true will make the task wait until work items have been completed and fail the build if work items fail. False is "fire and forget." | ||
| IsExternal: false # [DEPRECATED] -- doesn't do anything, jobs are external if HelixAccessToken is empty and Creator is set | ||
| HelixBaseUri: 'https://helix.dot.net/' # optional -- sets the Helix API base URI (allows targeting https://helix.int-dot.net ) | ||
| HelixBaseUri: 'https://helix.int-dot.net/' # optional -- sets the Helix API base URI (allows targeting https://helix.int-dot.net ) |
Copilot
AI
Feb 1, 2026
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.
The PR title explicitly states "NO-MERGE" indicating this is experimental and should not be merged into the main branch. This PR changes the Helix base URI from production (helix.dot.net) to staging (helix.int-dot.net), which would affect all Helix test submissions. Since this is marked as experimental in the description, it should not be merged without removing the NO-MERGE designation and proper validation.
| <HelixBaseUri>https://helix.int-dot.net/</HelixBaseUri> | ||
| <IncludeXHarnessCli>true</IncludeXHarnessCli> |
Copilot
AI
Feb 1, 2026
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.
The indentation of HelixBaseUri is inconsistent with other properties in this PropertyGroup. Other properties like IncludeXHarnessCli use 4 spaces of indentation, but HelixBaseUri uses 6 spaces. For consistency, HelixBaseUri should use 4 spaces of indentation to align with the existing properties in this file.
| <HelixBaseUri>https://helix.int-dot.net/</HelixBaseUri> | |
| <IncludeXHarnessCli>true</IncludeXHarnessCli> | |
| <HelixBaseUri>https://helix.int-dot.net/</HelixBaseUri> | |
| <IncludeXHarnessCli>true</IncludeXHarnessCli> |
|
|
||
| steps: | ||
| - powershell: 'powershell "$env:BUILD_SOURCESDIRECTORY\eng\common\msbuild.ps1 $env:BUILD_SOURCESDIRECTORY/${{ parameters.HelixProjectPath }} /restore /p:TreatWarningsAsErrors=false ${{ parameters.HelixProjectArguments }} /t:Test /bl:$env:BUILD_SOURCESDIRECTORY\artifacts\log\$env:BuildConfig\SendToHelix.binlog"' | ||
| - powershell: 'powershell "$env:BUILD_SOURCESDIRECTORY\eng\common\msbuild.ps1 $env:BUILD_SOURCESDIRECTORY/${{ parameters.HelixProjectPath }} /restore /p:HelixBaseUri=https://helix.int-dot.net/ /p:TreatWarningsAsErrors=false ${{ parameters.HelixProjectArguments }} /t:Test /bl:$env:BUILD_SOURCESDIRECTORY\artifacts\log\$env:BuildConfig\SendToHelix.binlog"' |
Copilot
AI
Feb 1, 2026
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.
The HelixBaseUri parameter is being passed as a command-line property here, but it's also set in the environment variables below (lines 59-60). This creates redundancy. Since the environment variable SYSTEM_HELIX_URI is being set, the command-line property may not be necessary.
| condition: and(${{ parameters.condition }}, eq(variables['Agent.Os'], 'Windows_NT')) | ||
| continueOnError: ${{ parameters.continueOnError }} | ||
| - script: $BUILD_SOURCESDIRECTORY/eng/common/msbuild.sh $BUILD_SOURCESDIRECTORY/${{ parameters.HelixProjectPath }} /restore /p:TreatWarningsAsErrors=false ${{ parameters.HelixProjectArguments }} /t:Test /bl:$BUILD_SOURCESDIRECTORY/artifacts/log/$BuildConfig/SendToHelix.binlog | ||
| - script: $BUILD_SOURCESDIRECTORY/eng/common/msbuild.sh $BUILD_SOURCESDIRECTORY/${{ parameters.HelixProjectPath }} /restore /p:TreatWarningsAsErrors=false ${{ parameters.HelixProjectArguments }} /p:HelixBaseUri=https://helix.int-dot.net/ /t:Test /bl:$BUILD_SOURCESDIRECTORY/artifacts/log/$BuildConfig/SendToHelix.binlog |
Copilot
AI
Feb 1, 2026
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.
The HelixBaseUri parameter is being passed as a command-line property here, but it's also set in the environment variables below (lines 90-91). This creates redundancy. Since the environment variable SYSTEM_HELIX_URI is being set, the command-line property may not be necessary.
experiment