Fix patch bug #25266
Fix patch bug #25266
Conversation
| @@ -244,7 +249,12 @@ if(!$BranchName) { | |||
|
|
|||
| try { | |||
| ## Creating a new branch | |||
| $cmdOutput = git checkout -b $BranchName $RemoteName/main | |||
| if($CreateNewBranch) { | |||
There was a problem hiding this comment.
Depending on the branch name you might hit issues if the branch already exists.
There was a problem hiding this comment.
Most likely you will hit those issues one push. You might need to harden the logic for retries, or potentially use our common script for that which already handles that.
There was a problem hiding this comment.
Do you mean challenges when we are pushing the changes to the upstream? Would we hit those even if we pushed using '-force' flag? Can you point me to the script?
My thought here was
(a) Most people would create a new branch for this.
(b) Some people may want to override by using special scenarios like storage - they use a single release tag to release all their SDK libraries. These people know what the expectations are from the branch, so will manually hold things through.
There was a problem hiding this comment.
The script I was referring to is at https://github.com/Azure/azure-sdk-for-java/blob/main/eng/common/scripts/git-branch-push.ps1.
We want to be careful about force pushing to a branch, especially in the main repo as someone might accidently give a branch with the same name as something else which could cause work to get lost. If we do force push I suggest we push to a fork and not the main branch (perhaps you already are but if not you should consider that).
There was a problem hiding this comment.
Got it. I actually try to push to the main repo since pipelines are rooted to it. Thanks for pointing out. I will remove the force push and use the common script instead.
There was a problem hiding this comment.
I looked at the script and I do not think I should use it in this scenario. I believe the common script is tackling a scenario where multiple people\teams could be pushing to the same upstream branch. while this script assumes it is running in isolation. I do think I should remove the force push. However I will err toward caution and let the script fail to push the changes if the remote branch exists.
There was a problem hiding this comment.
The common script is definitely more hardened to error cases and I do think it would work for your scenarios, but I'm also fine with you doing things explicitly here. If in when you need to make more updates to fix other scenarios you should consider using the common script.
There was a problem hiding this comment.
For example the common script will check to see if the branch exists and call the correct checkout command for either condition without needing to pass in a boolean.
There was a problem hiding this comment.
Yes. I think this script will be really helpful when we move to automating patch releases through pipeline but I don't think we are there yet. I want to spend a few more iterations before attempting that :)
|
Thank you for the review. |
* Fix patch bug * Disable force push and resetting common code quality files.
[Hub Generated] Review request for Microsoft.DocumentDB to add version stable/2023-09-15 (Azure#25266) * Adds base for updating Microsoft.DocumentDB from version stable/2023-04-15 to version 2023-09-15 * Updates readme * Updates API version in new specs and examples * burst capacity changes (Azure#25116) (Azure#25267) * burst capacity changes * Updated cosmos-db.json to include "type" of "Operation" * Update cosmos-db.json to reflect customerManagedKeyStatus. * lint fix (Azure#25336) * Fixed typo and removed semantic reference * Validation fix (Azure#25338) * lint fix * validation fix * Added CustomerManagedKeyStatus enum and correspondent references. * Update custom-words.txt * Update cosmos-db.json * rollback lint fix * suppressing avocado errors * Added space to rerun checks. * prettier fix * undo suppression as it is not working * Added clarification for throughput/autoscaleSettings option * removing added space --------- Co-authored-by: vchske <chskelt@microsoft.com> Co-authored-by: AdrianSibajaRetana <54075415+AdrianSibajaRetana@users.noreply.github.com> Co-authored-by: carjackson-msft <124716637+carjackson-msft@users.noreply.github.com>
Description
Add support for not creating a new branch for a patch release. This was needed by storage as they have multiple SDKs with the same release tag so in theory they can release all their libraries together using the single branch. Also fixed another bug in the script.