-
Notifications
You must be signed in to change notification settings - Fork 228
Using default branch for link checker #1458
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
Conversation
|
The following pipelines have been queued for testing: |
eng/common/pipelines/templates/steps/daily-dev-build-variable.yml
Outdated
Show resolved
Hide resolved
|
The following pipelines have been queued for testing: |
| Recursive: $false | ||
| CheckLinkGuidance: $true | ||
| Urls: '(Get-ChildItem -Path ./ -Recurse -Include *.md)' | ||
| BranchReplaceRegex: "^(${env:SYSTEM_PULLREQUEST_SOURCEREPOSITORYURI}.*/(?:blob|tree)/)$(DefaultBranch)(/.*)$" |
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.
You can still keep these here. The values will not get evaluated until the step that consumes them runs.
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.
Good to know. Will have it here and have a test.
| Condition: succeeded() # If you want to run on failure for the link checker, set it to `Condition: succeededOrFailed()`. | ||
|
|
||
| steps: | ||
| - pwsh: | |
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 like this step but I think we should put it into its own template.
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.
Sure. I am considering to do the same as we have other places need to use the template.
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
eng/common/scripts/Verify-Links.ps1
Outdated
| if (!$userAgent) { | ||
| $userAgent = "Chrome/87.0.4280.88" | ||
| } | ||
| if (!$branchReplaceRegex) { |
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.
Instead of calling git from this script I recommend we just remove the default value here as it isn't needed for basic usage anyway. We can simply add that information to the documentation for this parameter. We should actually also take this opportunity to use the PS help comment syntax to document this script. See https://docs.microsoft.com/en-us/powershell/scripting/developer/help/examples-of-comment-based-help?view=powershell-7.1#example-2-comment-based-help-for-a-script.
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 actually need to start doing that for more of my scripts as well :)
|
The following pipelines have been queued for testing: |
eng/common/scripts/Verify-Links.ps1
Outdated
| List of http status codes count as broken links. Defaults to 400, 401, 404, SocketError.HostNotFound = 11001, SocketError.NoData = 11004. | ||
|
|
||
| .PARAMETER branchReplaceRegex | ||
| Regex to check if the link needs to be replaced. |
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 give example of the regex here.
|
The following pipelines have been queued for testing: |
weshaggard
left a comment
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.
Lets wait to merge this until the end of the release week this week.
4f1cf9d to
70e13b8
Compare
|
The following pipelines have been queued for testing: |
|
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
The change here is to replace hard-coded master to default branch.
Testing pipeline: https://github.com/Azure/azure-sdk-for-java/pull/19589/files
Testing build on Pr validation: https://dev.azure.com/azure-sdk/public/_build/results?buildId=767101&view=results
Testing build on aggregate-report: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=767102&view=results