Skip to content
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

feat: Flag to disable custom [Atlantis] commit messages on PR merge #3120

Conversation

oliverisaac
Copy link
Contributor

@oliverisaac oliverisaac commented Feb 10, 2023

what

When PR's are merged in they are given the commit message:

[Atlantis] Automatically merging after successful apply

This makes it hard to track which PR applies to which commit.

This flag would allow us to turn off this behavior.

why

  • The custom [Atlantis] Automatically merging after successful apply message makes it hard to audit which PR applies to which commit.

tests

  • I duplicated tests related to the delete_source_branch_on_merge flag

references

The merge commit message is optional for gitlab and azdo:

GitLab: https://docs.gitlab.com/ee/api/merge_requests.html#merge-a-merge-request

AzDo: https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-requests/update?view=azure-devops-rest-5.1#gitpullrequestcompletionoptions

@oliverisaac oliverisaac requested a review from a team as a code owner February 10, 2023 17:47
@github-actions github-actions bot added docs Documentation go Pull requests that update Go code provider/azuredevops provider/gitlab labels Feb 10, 2023
@jamengual
Copy link
Contributor

isn't this possible with Custom templates? did you look into that?

@oliverisaac
Copy link
Contributor Author

I don't think so? The commit message is hard coded in the azure provider: https://github.com/runatlantis/atlantis/pull/3120/files#diff-58c908ca04501c732469c627f38c2df5c07a233e149bf7d49396dc2550c20d99L328

Unless custom templates would be used in another way?

@jamengual
Copy link
Contributor

@krrrr38 do you happen to know?

@krrrr38
Copy link
Contributor

krrrr38 commented Feb 11, 2023

Custom template manages only PR comments. Currently commit messages are not managed by template.


BTW how about add just pull number into commit message? New config seems to be too much.

example diff
diff --git a/server/events/vcs/common/common.go b/server/events/vcs/common/common.go
index 7a41b1d8..a1de298b 100644
--- a/server/events/vcs/common/common.go
+++ b/server/events/vcs/common/common.go
@@ -3,12 +3,15 @@
 package common

 import (
+       "fmt"
        "math"
 )

 // AutomergeCommitMsg is the commit message Atlantis will use when automatically
 // merging pull requests.
-const AutomergeCommitMsg = "[Atlantis] Automatically merging after successful apply"
+func AutomergeCommitMsg(pullNum int) string {
+       return fmt.Sprintf("[Atlantis] Automatically merging after successful apply #%d", pullNum)
+}

 // SplitComment splits comment into a slice of comments that are under maxSize.
 // It appends sepEnd to all comments that have a following comment.
diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go
index e530ab80..84715f43 100644
--- a/server/events/vcs/gitlab_client.go
+++ b/server/events/vcs/gitlab_client.go
@@ -295,7 +295,7 @@ func (g *GitlabClient) WaitForSuccessPipeline(ctx context.Context, pull models.P

 // MergePull merges the merge request.
 func (g *GitlabClient) MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error {
-       commitMsg := common.AutomergeCommitMsg
+       commitMsg := common.AutomergeCommitMsg(pull.Num)

@LanceSandino
Copy link

This would be a really cool feature to have 😄

@nitrocode
Copy link
Member

I agree with @krrrr38 . Instead of adding a new flag, it seems better to mention the PR number in the merge message.

What do you think @oliverisaac ?

@nitrocode nitrocode changed the title Add Flag to Disable Custom [Atlantis] commit messages on PR merge feat: Flag to disable custom [Atlantis] commit messages on PR merge Feb 22, 2023
@nitrocode nitrocode added the waiting-on-response Waiting for a response from the user label Feb 22, 2023
@oliverisaac
Copy link
Contributor Author

It seems better to mention the PR number in the merge message.

I'm totally okay with that answer: I was trying to maintain backwards compatibility but if we're okay with modifying the commit message I'm all for that :) Much easier!

@oliverisaac oliverisaac force-pushed the DEVOPS-674-atlantis-do-not-override-commit-message branch from 15aac1c to 15aace3 Compare February 28, 2023 16:36
@github-actions github-actions bot removed the docs Documentation label Feb 28, 2023
@oliverisaac
Copy link
Contributor Author

I've rebased the PR as discussed above

@oliverisaac oliverisaac force-pushed the DEVOPS-674-atlantis-do-not-override-commit-message branch from 15aace3 to 47f1c6f Compare February 28, 2023 16:38
oliverisaac and others added 2 commits March 16, 2023 16:39
Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>
@nitrocode
Copy link
Member

@oliverisaac please merge the default branch into the feature branch again in order to resolve the test failure

@oliverisaac
Copy link
Contributor Author

I've merged in main, thanks!

@nitrocode nitrocode added this to the v0.23.3 milestone Mar 20, 2023
@nitrocode nitrocode modified the milestones: v0.23.3, v0.23.4 Mar 20, 2023
@oliverisaac oliverisaac requested a review from nitrocode March 24, 2023 18:22
@nitrocode nitrocode enabled auto-merge (squash) March 25, 2023 02:02
@nitrocode
Copy link
Member

Thank you @oliverisaac !

@nitrocode nitrocode merged commit f35f4d6 into runatlantis:main Mar 25, 2023
nitrocode pushed a commit that referenced this pull request May 5, 2023
…#3120)

* Modify the automere commit message to include the PR number [DEVOPS-674]

* Update server/events/vcs/common/common.go

Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>

* added test for PR message func [DEVOPS-674]

---------

Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…runatlantis#3120)

* Modify the automere commit message to include the PR number [DEVOPS-674]

* Update server/events/vcs/common/common.go

Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>

* added test for PR message func [DEVOPS-674]

---------

Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
…runatlantis#3120)

* Modify the automere commit message to include the PR number [DEVOPS-674]

* Update server/events/vcs/common/common.go

Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>

* added test for PR message func [DEVOPS-674]

---------

Co-authored-by: Ken Kaizu <k.kaizu38@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code provider/azuredevops provider/gitlab waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants