Set default branch to master if not set from git #1558
Conversation
sima-zhu
added
the
Central-EngSys
|
The following pipelines have been queued for testing: |
| @@ -41,7 +41,7 @@ steps: | |||
|
|
|||
| - task: PowerShell@2 | |||
| displayName: Push changes | |||
| condition: and(succeeded(), eq(variables['HasChanges'], 'true')) | |||
| condition: and(succeeded(), ne(variables['HasChanges'], 'false')) | |||
There was a problem hiding this comment.
Why this change? We want to make sure we don't accidently start pushing these changes unless someone has explicitly set it to true.
There was a problem hiding this comment.
Several cases here:
- Step
Check for changesexecutes:
git diffsuccess: Then HasChange=true, we rungit pushon purpose.git difffail: Then HasChange=false, we skipgit pushon purpose.
- Step
Check for changesskips, then HasChanges not set, we will rungit pushstep anyway:
- If people intended to check in some commits to branch, then they have to literally put PushArgs: -f. (This is the purpose of the changes. In my case, I fetch some commits in master, and then merge the commits into live branch)
- If PushArgs not set, then the script will fail. We have to take care of this case which not fail previously.
There was a problem hiding this comment.
- That is has intended.
- If you set the value to skip the check for changes then you are in charge of setting HasChanges to true if there are changes that are interesting. We want to avoid blindly pushing changes in a case where someone doesn't set HasChanges at all.
As far as I'm aware this logic is working exactly has intended and if there is some scenario where you feel it is working please give me the specific case as I suspect that case needs to be address individually instead of changing the common template logic.
There was a problem hiding this comment.
Totally agree with you as the changes here might breaking some feature. I removed it from this PR. Will open a new one if it is needed.
| Write-Host "Setting DefaultBranch=$setDefaultBranch" | ||
| echo "##vso[task.setvariable variable=DefaultBranch]$setDefaultBranch" | ||
| displayName: "Setup Default Branch" | ||
| workingDirectory: ${{ parameters.workingDirectory }} | ||
| continueOnError: true |
There was a problem hiding this comment.
This will end up turning the pipeline yellow which we should try to avoid. Please do a test build on a private pipeline with this change and lets see how we can make this silent (i.e. no warnings/errors) only the write-host message.
There was a problem hiding this comment.
Will have a pipeline on my private fork repo
There was a problem hiding this comment.
|
The following pipelines have been queued for testing: |
|
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 (
|
ghost
merged commit 
Two changes in PR:
continueOnError: trueto unblock any failed places.E.g. https://dev.azure.com/azure-sdk/internal/_build/results?buildId=838864&view=logs&j=b70e5e73-bbb6-5567-0939-8415943fadb9&t=bdeefc16-b669-5ebd-ad94-a2c19ade53b0
Testing pipeline right here: https://dev.azure.com/azure-sdk/playground/_build/results?buildId=845015&view=logs&j=38d918fc-df5a-5870-2b80-6c17fdb13baf&t=489baed9-6f06-5266-9d0d-079a130738b4
2.
git-push-changes.ymlchecked whether local has changes. Make the changes so if we need to skip checking changes somehow, we are able to execute the scripts.E.g: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=842830&view=logs&j=321fe29c-d6e3-5c98-996f-e7c628f12971&t=71fd3457-22c6-5475-4ae6-23dcf07ca155