-
Notifications
You must be signed in to change notification settings - Fork 223
eng/common changes for docs metadata release #1725
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/update-docsms-metadata.yml
Outdated
Show resolved
Hide resolved
| $BranchName = "$(TargetBranchName)" | ||
| # Fetch and checkout remote branch if it already exists otherwise create a new branch. | ||
| git ls-remote --exit-code --heads $RemoteName $BranchName | ||
| if ($LASTEXITCODE -eq 0) { |
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 like you found a good solution. Should I close #1642 ?
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.
We've run into this problem in other parts of the codebase. @weshaggard has designs to pull this logic into a yaml template of sorts.
I think valid arguments can be made for including this in sparse-checkout.yml (it's easier to do everything in one template call as opposed to two) or not (call the second template when you need it, don't bloat sparse-checkout.yml).
One deciding factor could be whether we would use this kind of branch-querying logic in a context which does not use sparse checkout.
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.
Is what you're suggesting basically a "checkout main and then make new branch" option?
- template: sparse-checkout.yml
parameters:
NewBranch: <branch name>
We would also need to consider whether to support this parameter as global to the template and/or specific to each Repositories object.
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.
In the case where the branch does not exist, yes, check out the default some (probably the default) branch and then create the new branch. However the branch may already exist and in that case we want to just check out that branch.
|
The following pipelines have been queued for testing: |
08c52cd to
f9f57b7
Compare
|
The following pipelines have been queued for testing: |
|
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 be sure to verify this across the different languages as release week is here soon and we want to be sure we don't break folks.
|
The following pipelines have been queued for testing: |
|
All CIs have passed, all template releases (except Python) have passed. Python's template release is failing for reasons unrelated to this change. Ready to merge. |
|
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 (
|
| } | ||
|
|
||
| function GetRelativePath($path) { | ||
| # If the path is empty return an empty string |
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.
Do we have a case where we are validly passing empty here?
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.
In this instance Azure.Core.TestFramework does not have a CHANGELOG.md https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/core/Azure.Core.TestFramework
In previous builds this would have produced output like:
{
"Name": "Azure.Core.TestFramework",
"Version": "",
"DirectoryPath": "D:\\a\\_work\\1\\s\\sdk\\core\\Azure.Core.TestFramework",
"ServiceDirectory": "core",
"ReadMePath": "D:\\a\\_work\\1\\s\\sdk\\core\\Azure.Core.TestFramework\\README.md",
"ChangeLogPath": "",
"Group": null,
"SdkType": "client",
"IsNewSdk": true,
"ArtifactName": "Azure.Core.TestFramework",
"ReleaseStatus": null
}
Original PR: Azure/azure-sdk-for-js#15749