-
Notifications
You must be signed in to change notification settings - Fork 183
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
Update eng/common workflow docs #1130
Conversation
The following pipelines have been queued for testing: |
7. Go review and approve each of your **Sync PRs**. The merging will happen automatically. | ||
8. Sign off on VerifyAndMerge stage of the sync pipeline using the approval gate. This stage will merge any remaining open **Sync PRs** and also append `auto-merge` to the **Tools PR** so it will automatically merge once the pipeline finishes. | ||
7. Sign Off on the VerifyAndMerge stage. This will merge any remaining open **Sync PR** and also append `auto-merge` to the **Tools PR**. | ||
6. Sign off on CreateSyncPRs stage of the sync pipeline using the approval gate. This stage will create the **Sync PRs** in the various language repos. A link to each of the **Sync PRs** will show up in the **Tools PR** for you to click and review. |
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.
Currently it seems like we need to also do a release of the template in order to get the Sync PR's green in the languages we use the template pipelines. Is this something you are still trying to figure out? If it is something that is required we should add that information in these docs.
cc @benbp
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 found a few times today where I was going through the motions of doing a release even though I wasn't actually touching anything that impacted the release pipeline directly. I think we should document the break glass procedure for these scenarios (e.g. merging the PRs even without the pipelines completing). It requires admin rights which means it will get some scrutiny, except when we do it :|
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.
The other break glass option is to override check-enforcer. However I think I would like to see if we can figure out a way to not have this block.
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.
It should be fixed by these PRs.
Azure/azure-sdk-for-net#16145
Azure/azure-sdk-for-js#11986
Azure/azure-sdk-for-java#16657
Azure/azure-sdk-for-python#14686
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 any end-to-end tests that verify we no longer block the sync PRs?
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.
Am testing now with another change.
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 assume your tests came back positive? So the workflow no longer blocks if a release isn't approved? I ask because it does look like you still needed to do a check-enforcer override in the sync PRs.
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 override the yaml configuration on the pipeline. So I added a manual branch exclusion to the pipeline UI. In my test it still added the internal template run as part of the PR checks, but it showed up green and did not block the pipeline. I still need more example tp know if that will be its consistent behavior.
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 created a separate issue to track what I believe is being discussed here:
#1147 Sync PRs should be auto-mergable if check-enforcer is green
c67f88a
to
7822ab6
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 (
|
Update eng/common workflow.