-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
AzureDevops: fix automerge #1186
Conversation
owner, project, repoName := SplitAzureDevopsRepoFullName(pull.BaseRepo.FullName) | ||
mergeResult, _, err := g.Client.PullRequests.Merge( | ||
g.ctx, | ||
owner, | ||
project, | ||
repoName, | ||
pull.Num, | ||
mergePull, |
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.
Oh, and mergePull isn't actually used by the g.Client.PullRequests.Merge function call so we can set that to nil. If you want this could be in a separate PR, I feel that dropping the unnecessary setup of mergePull helps clarify the code.
Codecov Report
@@ Coverage Diff @@
## master #1186 +/- ##
==========================================
- Coverage 70.71% 70.53% -0.18%
==========================================
Files 71 71
Lines 5975 5983 +8
==========================================
- Hits 4225 4220 -5
- Misses 1398 1408 +10
- Partials 352 355 +3
Continue to review full report at Codecov.
|
@oliverisaac I am the colleague of the person who opened this. We fixed it in our internal fork which I offered amonst other fixes in #1189 . Could you let me know if I went down the right path? It appears to be working for us internally. Microsoft Graph API descriptor objects are non-obvious and hard 😂 |
@zparnold , I love some of the changes y'all made, Especially adding the option to delete a source branch on merge! I tried to close the branch using the GUID of the PR owner, but the PAT we gave to the atlantis app doesn't have permission to impersonate users and so wasn't able to close using anything but its own GUID. We set up a user account which acts as a "service account" in azdo and then generated a PAT in that user's profile so that we could lock down the account as much as possible. Any idea which permissions you gave your PAT so it could close as someone else? |
@Dilregore might be the person from our side who can answer that. I just fix the bugs he observes 😂 |
We created another PR which is using a brand new feature we added to the SDK to query the user ID dynamically. |
#1289 solved this. You should probably abandon @oliverisaac |
I'll close this then but if I'm wrong to have closed it, let us know and we'll reopen. |
As covered in #1152 , the user GUID must be set for the auto merge in Azure Devops to actually run.
@lkysow , this is a "hacky" solution to that problem. This is a proof-of-concept (though fully functional) PR. If you're okay with the direction this is headed I can add the expected/required config options/documentation.I got carried away and this is a full PR now :)