-
Notifications
You must be signed in to change notification settings - Fork 379
Fix Helix URLs #15672
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
Fix Helix URLs #15672
Conversation
Fix Helix job's double slash which prevents users from navigating directly to the target.
| </PropertyGroup> | ||
| <Message Condition=" '$(HelixBaseUri)' == '' " Text="Sent Helix Job; see work items at https://helix.dot.net/api/jobs/$(HelixJobId)/workitems?api-version=2019-06-17$(_AccessTokenSuffix)" Importance="High" /> | ||
| <Message Condition=" '$(HelixBaseUri)' != '' " Text="Sent Helix Job; see work items at $(HelixBaseUri)/api/jobs/$(HelixJobId)/workitems?api-version=2019-06-17$(_AccessTokenSuffix)" Importance="High" /> | ||
| <Message Condition=" '$(HelixBaseUri)' != '' " Text="Sent Helix Job; see work items at $(HelixBaseUri)$(HelixBaseUri.EndsWith('/') ? '' : '/')api/jobs/$(HelixJobId)/workitems?api-version=2019-06-17$(_AccessTokenSuffix)" Importance="High" /> |
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 don't want to make any assumptions whether HelixBaseUri is "/"-terminated or not. Though, it is certainly an option.
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.
Files not reviewed (1)
- src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MonoQueue.targets: Language not supported
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 for the change but I don't think it's implemented well.
Possibly You might need to add 2 message calls based on the EndsWith where one will include the slash and one won't
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MonoQueue.targets
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MonoQueue.targets
Outdated
Show resolved
Hide resolved
…k.MonoQueue.targets
|
Thank you for your help! |
Fix Helix job's double slash which prevents users from navigating directly to the target.
Currently