feat!: Add merge queue parameters to repository ruleset #3253

zepeng811 · 2024-08-28T21:05:21Z

Fixes: #3225.

BREAKING CHANGE: NewMergeQueueRule now takes one parameter: *MergeQueueRuleParameters.

Summary

closes: #3225
(I'm the same author as the assignee of the issue @zhpeng811, but submitting the
PR using my corporate account)

Details

This PR adds merge queue parameter configurations to the repository branch ruleset.

an attempt was also made for organization ruleset but was later determined that GitHub currently does not support merge queue configurations at the organizational level. Configs related to merge queue in tests was removed as the result.

Testing

General

Ran the required scripts on step 4 of the CONTRIBUTING.md file

Details

❯ script/fmt.sh
❯ script/test.sh
testing .
?       github.com/google/go-github/v63/test/integration        [no test files]
ok      github.com/google/go-github/v63/github  (cached)        coverage: 99.4% of statements
        github.com/google/go-github/v63/test/fields             coverage: 0.0% of statements
testing example
        github.com/google/go-github/v63/example/actionpermissions               coverage: 0.0% of statements
        github.com/google/go-github/v63/example/basicauth               coverage: 0.0% of statements
        github.com/google/go-github/v63/example/codespaces/newusersecretwithxcrypto             coverage: 0.0% of statements
        github.com/google/go-github/v63/example/codespaces/newreposecretwithxcrypto             coverage: 0.0% of statements
        github.com/google/go-github/v63/example/appengine               coverage: 0.0% of statements
        github.com/google/go-github/v63/example/newfilewithappauth              coverage: 0.0% of statements
        github.com/google/go-github/v63/example/listenvironments                coverage: 0.0% of statements
        github.com/google/go-github/v63/example/commitpr                coverage: 0.0% of statements
        github.com/google/go-github/v63/example/migrations              coverage: 0.0% of statements
        github.com/google/go-github/v63/example/newrepo         coverage: 0.0% of statements
        github.com/google/go-github/v63/example/newreposecretwithxcrypto                coverage: 0.0% of statements
        github.com/google/go-github/v63/example/simple          coverage: 0.0% of statements
        github.com/google/go-github/v63/example/ratelimit               coverage: 0.0% of statements
        github.com/google/go-github/v63/example/tagprotection           coverage: 0.0% of statements
        github.com/google/go-github/v63/example/tokenauth               coverage: 0.0% of statements
        github.com/google/go-github/v63/example/topics          coverage: 0.0% of statements
testing scrape
ok      github.com/google/go-github/scrape      (cached)        coverage: 59.4% of statements
        github.com/google/go-github/scrape/example/scrape               coverage: 0.0% of statements
testing tools
ok      tools/metadata  (cached)        coverage: 81.7% of statements
❯ script/lint.sh
linting .
linting example
linting scrape
linting tools
validating generated files

Repository Ruleset

Manual:
Tested with the following test code, both with and without including the specific parameters, and verified in the UI.

Expand to view code

func main() {
	client := github.NewClient(nil).WithAuthToken(os.Getenv("GITHUB_TOKEN"))
	org := "<REPLACE_WITH_YOUR_ORG>"
	repo := "<REPLACE_WITH_YOUR_REPO>"

	ctx := context.Background()
	_, _, err := client.Repositories.CreateRuleset(ctx, org, repo, &github.Ruleset{
		Name:        "merge-queue-test-without-param",
		Enforcement: "active",
		Rules: []*github.RepositoryRule{
			github.NewMergeQueueRule(nil),
		},
	})
	if err != nil {
		fmt.Println(err)
	}
	_, _, err = client.Repositories.CreateRuleset(ctx, org, repo, &github.Ruleset{
		Name:        "merge-queue-test-with-param",
		Enforcement: "active",
		Rules: []*github.RepositoryRule{
			github.NewMergeQueueRule(&github.MergeQueueRuleParameters{
				CheckResponseTimeoutMinutes:  35,
				GroupingStrategy:             "HEADGREEN",
				MaxEntriesToBuild:            8,
				MaxEntriesToMerge:            4,
				MergeMethod:                  "SQUASH",
				MinEntriesToMerge:            2,
				MinEntriesToMergeWaitMinutes: 13,
			}),
		},
	})
	if err != nil {
		fmt.Println(err)
	}
}

Unit Test:
A unit test already exist for merge queue without parameters, 2 more tests were added (1 success case and 1 failure case with the incorrect parameter type)

Organization Ruleset

Was trying to create a org ruleset with merge queue rule using the following curl

Expand for curl

export ORG_NAME=<REPLACE_WITH_YOUR_ORG_NAME>

curl -L \
  -X POST \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer $GITHUB_TOKEN" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://github.com/gitapi/orgs/$ORG_NAME/rulesets \
  -d '{"name":"merge queue test ruleset","target":"branch","enforcement":"active","conditions":{"ref_name":{"include":["refs/heads/main","refs/heads/master"],"exclude":["refs/heads/dev*"]},"repository_name":{"include":["test1","test2"],"exclude":["test3"],"protected":true}},"rules":[{"type":"merge_queue"}]}'

but got the following error:

{
  "message": "Validation Failed",
  "errors": [
    "Invalid rules: 'Merge queue'"
  ],
  "documentation_url": "https://docs.github.com/rest/orgs/rules#create-an-organization-repository-ruleset",
  "status": "422"
}

I cross referenced with the UI page and did not notice a branch rule for merge queue, which conflicts with the REST API documentation for org rulesets, so seems like GitHub currently does not support merge queue configuration on the org level ruleset. Reported the issue to GitHub community: https://github.com/orgs/community/discussions/137097

codecov · 2024-08-28T21:19:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.94%. Comparing base (2b8c7fa) to head (02f3dc8).
Report is 105 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3253      +/-   ##
==========================================
- Coverage   97.72%   92.94%   -4.78%     
==========================================
  Files         153      171      +18     
  Lines       13390    11663    -1727     
==========================================
- Hits        13085    10840    -2245     
- Misses        215      729     +514     
- Partials       90       94       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gmlewis · 2024-08-28T21:23:19Z

github/repos_rules.go

@@ -110,6 +110,19 @@ type RuleRequiredStatusChecks struct {
 	IntegrationID *int64 `json:"integration_id,omitempty"`
 }

+// MergeQueueRuleParameters represents the merge_queue rule parameters.
+type MergeQueueRuleParameters struct {


@zepeng811 - should any of these parameters be optional (and therefore pointers with the omitempty JSON tag added)?

@gmlewis did a quick test and seems like it must be all or nothing

adding all parameters like this worked:

curl -L \ -X POST \ -H "Accept: application/vnd.github+json" \ -H "Authorization: Bearer $GITHUB_TOKEN" \ -H "X-GitHub-Api-Version: 2022-11-28" \ https://github.com/gitapi/repos/$ORG/$REPO/rulesets \ -d '{"name":"merge queue test ruleset","target":"branch","enforcement":"active","conditions":{"ref_name":{"include":["refs/heads/main","refs/heads/master"],"exclude":["refs/heads/dev*"]}},"rules":[{"type":"merge_queue","parameters":{"check_response_timeout_minutes":35,"grouping_strategy":"HEADGREEN","max_entries_to_build":8,"max_entries_to_merge":4,"merge_method":"SQUASH","min_entries_to_merge":2,"min_entries_to_merge_wait_minutes":13}}]}'

but if any parameter was removed, like:

curl -L \ -X POST \ -H "Accept: application/vnd.github+json" \ -H "Authorization: Bearer $GITHUB_TOKEN" \ -H "X-GitHub-Api-Version: 2022-11-28" \ https://github.com/gitapi/repos/$ORG/$REPO/rulesets \ -d '{"name":"merge queue test ruleset 2","target":"branch","enforcement":"active","conditions":{"ref_name":{"include":["refs/heads/main","refs/heads/master"],"exclude":["refs/heads/dev*"]}},"rules":[{"type":"merge_queue","parameters":{"check_response_timeout_minutes":35,"grouping_strategy":"HEADGREEN","max_entries_to_build":8,"max_entries_to_merge":4,"merge_method":"SQUASH","min_entries_to_merge":2}}]}'

then the API will error:

{ "message": "Invalid request.\n\nInvalid property /rules/0: data matches no possible input. See `documentation_url`.", "documentation_url": "https://docs.github.com/rest/repos/rules#create-a-repository-ruleset", "status": "422" }

OK, thanks for testing!

gmlewis

Thank you, @zepeng811 !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

gmlewis · 2024-09-03T18:13:01Z

Thank you, @ali-kafel and @tomleicircle !
Merging.

zepeng811 added 4 commits August 15, 2024 09:37

add merge queue parameters to ruleset

636b6a5

remove merge queue from org ruleset test

2c6e794

add a test with incorrect parameter type

1cf99cb

Merge branch 'master' into issue_3225

02f3dc8

zepeng811 marked this pull request as draft August 28, 2024 21:16

gmlewis changed the title ~~feat: add merge queue parameters to repository ruleset~~ feat!: Add merge queue parameters to repository ruleset Aug 28, 2024

gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Aug 28, 2024

gmlewis reviewed Aug 28, 2024

View reviewed changes

zepeng811 marked this pull request as ready for review August 28, 2024 21:23

gmlewis approved these changes Aug 28, 2024

View reviewed changes

ali-kafel approved these changes Sep 3, 2024

View reviewed changes

tomleicircle approved these changes Sep 3, 2024

View reviewed changes

gmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 3, 2024

gmlewis merged commit 00caddc into google:master Sep 3, 2024
6 of 7 checks passed

This was referenced Sep 4, 2024

[FEAT]: Add support for Merge queue on github. integrations/terraform-provider-github#1481

Open

feat: add support for merge queues in repository and organization rulesets integrations/terraform-provider-github#2380

Open

gmlewis mentioned this pull request Sep 16, 2024

Bump version of go-github to v65.0.0 #3266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Add merge queue parameters to repository ruleset #3253

feat!: Add merge queue parameters to repository ruleset #3253

zepeng811 commented Aug 28, 2024 •

edited

Loading

codecov bot commented Aug 28, 2024 •

edited

Loading

gmlewis Aug 28, 2024

zepeng811 Aug 28, 2024

gmlewis Aug 28, 2024

gmlewis left a comment

gmlewis commented Sep 3, 2024

feat!: Add merge queue parameters to repository ruleset #3253

feat!: Add merge queue parameters to repository ruleset #3253

Conversation

zepeng811 commented Aug 28, 2024 • edited Loading

Summary

Details

Testing

General

Repository Ruleset

Organization Ruleset

codecov bot commented Aug 28, 2024 • edited Loading

Codecov Report

gmlewis Aug 28, 2024

Choose a reason for hiding this comment

zepeng811 Aug 28, 2024

Choose a reason for hiding this comment

gmlewis Aug 28, 2024

Choose a reason for hiding this comment

gmlewis left a comment

Choose a reason for hiding this comment

gmlewis commented Sep 3, 2024

zepeng811 commented Aug 28, 2024 •

edited

Loading

codecov bot commented Aug 28, 2024 •

edited

Loading