Skip to content

Commit 08c4894

Browse files
committed
[github] Don't consider PR body in CI skip logic
GitHub pull request edits were not triggering CI when the PR body/description contained the `[ ci skip ]`/`[ skip ci ]` pattern. This was an issue, for example, in Dependabot-generated PRs that include commit messages from upstream dependencies that do include the pattern. Furthermore, no other CI service or vendor considers the PR body for the "ci skip" mechanism. This corrects the behavior so that only commit messages or PR titles are considered when a PR is edited.
1 parent de1770c commit 08c4894

File tree

2 files changed

+18
-26
lines changed

2 files changed

+18
-26
lines changed

service/hook/github/github.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,19 @@ func transformPullRequestEvent(pullRequest PullRequestEventModel) hookCommon.Tra
181181
}
182182
}
183183
if pullRequest.Action == "edited" {
184-
// skip it if only title / description changed, and the previous pattern did not include a [skip ci] pattern
184+
// skip it if only title / description changed, and the current title did not remove a [skip ci] pattern
185185
if pullRequest.Changes.Base == nil {
186-
if !hookCommon.IsSkipBuildByCommitMessage(pullRequest.Changes.Title.From) && !hookCommon.IsSkipBuildByCommitMessage(pullRequest.Changes.Body.From) {
186+
// only description changed
187+
if pullRequest.Changes.Title.From == "" {
187188
return hookCommon.TransformResultModel{
188-
Error: errors.New("Pull Request edit doesn't require a build: only title and/or description was changed, and previous one was not skipped"),
189+
Error: errors.New("Pull Request edit doesn't require a build: only body/description was changed"),
190+
ShouldSkip: true,
191+
}
192+
}
193+
// title changed without removing any [skip ci] pattern
194+
if !hookCommon.IsSkipBuildByCommitMessage(pullRequest.Changes.Title.From) {
195+
return hookCommon.TransformResultModel{
196+
Error: errors.New("Pull Request edit doesn't require a build: only title was changed, and previous one was not skipped"),
189197
ShouldSkip: true,
190198
}
191199
}

service/hook/github/github_test.go

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,7 @@ func Test_transformPullRequestEvent(t *testing.T) {
763763
},
764764
}
765765
hookTransformResult := transformPullRequestEvent(pullRequest)
766-
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title and/or description was changed, and previous one was not skipped")
766+
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title was changed, and previous one was not skipped")
767767
require.True(t, hookTransformResult.ShouldSkip)
768768
require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams)
769769
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
@@ -828,7 +828,7 @@ func Test_transformPullRequestEvent(t *testing.T) {
828828
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
829829
}
830830

831-
t.Log("Pull Request - edited - only body/description change - no skip ci in previous - no build")
831+
t.Log("Pull Request - edited - only body/description change - no build")
832832
{
833833
pullRequest := PullRequestEventModel{
834834
Action: "edited",
@@ -865,13 +865,13 @@ func Test_transformPullRequestEvent(t *testing.T) {
865865
},
866866
}
867867
hookTransformResult := transformPullRequestEvent(pullRequest)
868-
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title and/or description was changed, and previous one was not skipped")
868+
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only body/description was changed")
869869
require.True(t, hookTransformResult.ShouldSkip)
870870
require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams)
871871
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
872872
}
873873

874-
t.Log("Pull Request - edited - only body/description change - BUT the previous title included a skip CI pattern - should build")
874+
t.Log("Pull Request - edited - only body/description change - the previous body included a skip CI pattern - still shouldn't build")
875875
{
876876
pullRequest := PullRequestEventModel{
877877
Action: "edited",
@@ -908,25 +908,9 @@ func Test_transformPullRequestEvent(t *testing.T) {
908908
},
909909
}
910910
hookTransformResult := transformPullRequestEvent(pullRequest)
911-
require.NoError(t, hookTransformResult.Error)
912-
require.False(t, hookTransformResult.ShouldSkip)
913-
require.Equal(t, []bitriseapi.TriggerAPIParamsModel{
914-
{
915-
BuildParams: bitriseapi.BuildParamsModel{
916-
CommitHash: "83b86e5f286f546dc5a4a58db66ceef44460c85e",
917-
CommitMessage: "PR test\n\nPR text body",
918-
Branch: "feature/github-pr",
919-
BranchDest: "develop",
920-
PullRequestID: pointers.NewIntPtr(12),
921-
PullRequestRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git",
922-
BaseRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git",
923-
HeadRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git",
924-
PullRequestMergeBranch: "pull/12/merge",
925-
PullRequestHeadBranch: "pull/12/head",
926-
Environments: make([]bitriseapi.EnvironmentItem, 0),
927-
},
928-
},
929-
}, hookTransformResult.TriggerAPIParams)
911+
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only body/description was changed")
912+
require.True(t, hookTransformResult.ShouldSkip)
913+
require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams)
930914
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
931915
}
932916
}

0 commit comments

Comments
 (0)