Skip to content

Commit 54da7a2

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 7e68fd7 commit 54da7a2

File tree

2 files changed

+18
-25
lines changed

2 files changed

+18
-25
lines changed

service/hook/github/github.go

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

service/hook/github/github_test.go

+7-22
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ func Test_transformPullRequestEvent(t *testing.T) {
654654
},
655655
}
656656
hookTransformResult := transformPullRequestEvent(pullRequest)
657-
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")
657+
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only title was changed, and previous one was not skipped")
658658
require.True(t, hookTransformResult.ShouldSkip)
659659
require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams)
660660
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
@@ -717,7 +717,7 @@ func Test_transformPullRequestEvent(t *testing.T) {
717717
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
718718
}
719719

720-
t.Log("Pull Request - edited - only body/description change - no skip ci in previous - no build")
720+
t.Log("Pull Request - edited - only body/description change - no build")
721721
{
722722
pullRequest := PullRequestEventModel{
723723
Action: "edited",
@@ -753,13 +753,13 @@ func Test_transformPullRequestEvent(t *testing.T) {
753753
},
754754
}
755755
hookTransformResult := transformPullRequestEvent(pullRequest)
756-
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")
756+
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only body/description was changed")
757757
require.True(t, hookTransformResult.ShouldSkip)
758758
require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams)
759759
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
760760
}
761761

762-
t.Log("Pull Request - edited - only body/description change - BUT the previous title included a skip CI pattern - should build")
762+
t.Log("Pull Request - edited - only body/description change - the previous body included a skip CI pattern - still shouldn't build")
763763
{
764764
pullRequest := PullRequestEventModel{
765765
Action: "edited",
@@ -795,24 +795,9 @@ func Test_transformPullRequestEvent(t *testing.T) {
795795
},
796796
}
797797
hookTransformResult := transformPullRequestEvent(pullRequest)
798-
require.NoError(t, hookTransformResult.Error)
799-
require.False(t, hookTransformResult.ShouldSkip)
800-
require.Equal(t, []bitriseapi.TriggerAPIParamsModel{
801-
{
802-
BuildParams: bitriseapi.BuildParamsModel{
803-
CommitHash: "83b86e5f286f546dc5a4a58db66ceef44460c85e",
804-
CommitMessage: "PR test\n\nPR text body",
805-
Branch: "feature/github-pr",
806-
BranchDest: "develop",
807-
PullRequestID: pointers.NewIntPtr(12),
808-
PullRequestRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git",
809-
BaseRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git",
810-
HeadRepositoryURL: "https://github.com/bitrise-io/bitrise-webhooks.git",
811-
PullRequestMergeBranch: "pull/12/merge",
812-
PullRequestHeadBranch: "pull/12/head",
813-
},
814-
},
815-
}, hookTransformResult.TriggerAPIParams)
798+
require.EqualError(t, hookTransformResult.Error, "Pull Request edit doesn't require a build: only body/description was changed")
799+
require.True(t, hookTransformResult.ShouldSkip)
800+
require.Equal(t, []bitriseapi.TriggerAPIParamsModel(nil), hookTransformResult.TriggerAPIParams)
816801
require.Equal(t, false, hookTransformResult.DontWaitForTriggerResponse)
817802
}
818803
}

0 commit comments

Comments
 (0)