MSCI Various Fixes #1189
MSCI Various Fixes #1189
Conversation
[skip ci]
initial attempt to fix pr merge problems
…urable add initial attempt
initial tested fix
|
Thanks for the PR! @mcdafydd are you free to review this? I'm not sold on the delete source branch feature, that seems like it doesn't belong in Atlantis, rather in the VCS itself or some other automation. |
|
@lkysow totally understand the delete source branch feature suggestion. The reason we built it is because Azure DevOps and GitLab both have an "Autocomplete" feature where a PR requestor can set the PR to auto merge and delete the source branch (optionally) when that happens. We happen to use it extensively at our company to keep git clean but it's not for everyone. BTW, if you are looking for folks to help maintain Azure DevOps related functionality in Atlantis I think we'd be interested. We use this pretty heavily internally and are vested in having our devs have a solid experience. LMK :) |
|
Yeah I think it's one of those things that would be super useful but I'm a bit worried about the maintenance workload. However it's essentially the same code path as automerge so shouldn't be too bad. Yes I'm sure @mcdafydd and I would love some more AZDO maintainers! |
| @@ -438,6 +434,7 @@ func (p *DefaultProjectCommandBuilder) buildCtx(ctx *CommandContext, | |||
| BaseRepo: ctx.BaseRepo, | |||
| EscapedCommentArgs: p.escapeArgs(commentArgs), | |||
| AutomergeEnabled: automergeEnabled, | |||
| DeleteSourceBranchOnMerge: deleteBranchOnMerge, | |||
|
We also use the delete source branch feature for merging PRs. Nice addition, IMHO. |
|
Is any assistance needed to move this PR along? Unfortunately, we found ourselves fighting against #1172 this morning and stumbled onto this PR. Happy to lend a hand pushing this over the goal line if you are looking for assistance. |
| @@ -56,24 +55,23 @@ func (d *DefaultCommitStatusUpdater) UpdateCombined(repo models.Repo, pull model | |||
| descripWords = "succeeded." | |||
| } | |||
| descrip := fmt.Sprintf("%s %s", strings.Title(command.String()), descripWords) | |||
| return d.Client.UpdateStatus(repo, pull, status, src, descrip, "") | |||
| return d.Client.UpdateStatus(repo, pull, status, command.String(), descrip, "") | |||
There was a problem hiding this comment.
This will change the form of the src argument from atlantis/<command> to just the command name. This src field is used by most of the VCS plugins to drive a data field that is sent along in the PR status update requests. Changing the format of this field is going to be a breaking change to anyone with a PR policy keyed off of this particular string.
There was a problem hiding this comment.
We've been fighting #1172 as well and it's gotten to a point where we've had to disable the status policies altogether and rely on reviewers only. I'd like to 2nd what @acastle said, if I can help in any way to getting this merged let me know.
Would it make sense to use a different string just for DevOps users? It's already broken for them anyway and prevents breaking changes to other VCS users.
|
Disappointingly I need to abandon this PR as it seems our internal implementation is destined to diverge a bit from the open source equivalent, please feel free to use and repurpose code from this PR though! |
This is based on an abandoned PR (runatlantis#1189) from Zack Arnold (zparnold).
|
Hey, Yeah so the problem is that the SDK needs to be changed in order to get the Bot user's ID dynamically during the runtime to be able to merge. We have a new colleague in our team and he will start to work on it and hopefully he will be able to open a PR for the SDK soon. Once that is done & merged we are planning to correct the function where before we were trying to use the ID of the PR opener which was not working. We have a working version of the "deleteSourceBranchOnMerge" which is overridable on 3 levels (server, repo, project) what we plan to add once merge is working properly. Stay tuned! :-) |
This is based on an abandoned PR (runatlantis#1189) from Zack Arnold (zparnold).
This is based on an abandoned PR (runatlantis#1189) from Zack Arnold (zparnold).
This is based on an abandoned PR (runatlantis#1189) from Zack Arnold (zparnold).
Hi there, this is one pretty big PR and feel free to pick and choose but we've been running this in our Azure DevOps tenant for a while and fixed things we figured we could contribute :) Addressed:
Feel free to reject this PR and tell us what you want broken down into pieces