Skip to content

Conversation

@JiayangZhou
Copy link
Contributor

@JiayangZhou JiayangZhou commented Dec 10, 2025

BREAKING CHANGE: CopilotCodeReviewRuleParameters.ReviewNewPushes is now ReviewOnPush.

Rename ReviewNewPushes to ReviewOnPush with json tag review_on_push

Hey sorry when I reviewed the github API doc https://docs.github.com/en/rest/repos/rules?apiVersion=2022-11-28 I noticed that the payload parameter we previously used was incorrect, thus I am creating this PR to correct it, sorry for the innocence!

Edit: also added MarshalJSON tests under func TestRepositoryRule. currently only for copilot_code_review 'creation' i guess others can add if they want

Edit2:
I think you are right from the beginning, i was too naive, so for this API

func (s *RepositoriesService) GetRulesForBranch(ctx context.Context, owner, repo, branch string, opts *ListOptions) (*BranchRules, *Response, error) {
the respond body is Unmarshaling to BranchRules and it does include codepilot,
i verified it from here too https://docs.github.com/en/rest/repos/rules?apiVersion=2022-11-28#get-rules-for-a-branch

@codecov
Copy link

codecov bot commented Dec 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.44%. Comparing base (1830689) to head (b4c3e16).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3874   +/-   ##
=======================================
  Coverage   92.44%   92.44%           
=======================================
  Files         200      200           
  Lines       14494    14500    +6     
=======================================
+ Hits        13399    13405    +6     
  Misses        895      895           
  Partials      200      200           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmlewis gmlewis changed the title Fix: copilot_code_review parameter names to match GitHub API fix!: Change copilot_code_review field names to match GitHub API Dec 10, 2025
@gmlewis 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 Dec 10, 2025
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @JiayangZhou!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @alexandear - @zyfy29

Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Dec 11, 2025
@gmlewis
Copy link
Collaborator

gmlewis commented Dec 11, 2025

Thank you, @stevehipwell!
Merging.

@gmlewis gmlewis merged commit b7fec34 into google:master Dec 11, 2025
7 checks passed
@mkushakov
Copy link

I noticed that the payload parameter we previously used was incorrect

@JiayangZhou It is not your fault. GitHub just changed payload params for Copilot Code Review to move it from Pull Request property boolean to Rules root object without bumping API version last week. We encountered same issue. And since this field was in Preview, they didn't even bother to announce breaking change API.

@stevehipwell
Copy link
Contributor

It's a good job they added API versioning... 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants