Skip to content

Fix repo rules bypass settings - #2831

Merged
gmlewis merged 3 commits into
google:masterfrom
o-sama:fix-repo-rules
Jul 18, 2023
Merged

Fix repo rules bypass settings#2831
gmlewis merged 3 commits into
google:masterfrom
o-sama:fix-repo-rules

Conversation

@o-sama

@o-samao-sama commented Jul 15, 2023

Copy link
Copy Markdown
Contributor

This PR reconciles library with GitHub API for the following regarding repo rulesets:

  • BypassMode should be part of BypassActor.
  • ActorType possible values documentation updated to match API docs.
  • `BypassMode possible values documentation updated to match API docs.
  • Ruleset ID field changed to Int64 pointer and added omitempty to allow for nil, ruleset ID is unknown prior to creation and is not required when sending a POST request to create ruleset.

@codecov

codecovBot commented Jul 15, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2831 (889737e) into master (bedd4e3) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 889737e differs from pull request most recent head cb37562. Consider uploading reports for the commit cb37562 to get more accurate results

@@ Coverage Diff @@## master #2831 +/- ##
==========================================
- Coverage 98.06% 98.06% -0.01% 
==========================================
Files 137 137 Lines 12289 12287 -2 ==========================================
- Hits 12051 12049 -2 
Misses 162 162 Partials 76 76 
Impacted FilesCoverage Δ
github/repos_rules.go100.00% <ø> (ø)

... and 3 files with indirect coverage changes

@gmlewisgmlewis changed the title fix: repo rules bypass settingsFix repo rules bypass settingsJul 15, 2023
@gmlewisgmlewis 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 Jul 15, 2023
Comment threadgithub/repos_rules.go

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @o-sama !
LGTM.

@liaodaniel - could you please look at these breaking API changes since you are the original author of this section of code and make sure you are happy with them?

@o-sama

o-sama commented Jul 17, 2023

Copy link
Copy Markdown
ContributorAuthor

It seems the breaking change was introduced by GitHub (point 2 here): https://github.blog/changelog/2023-06-27-repository-rules-public-beta-updates/

The latest change in master added a couple of test cases related to organization rulesets which used Int64 as opposed to pointers in ruleset ID, this was the case prior to the change I’m proposing here so I fixed that in the latest commit.

@liaodaniel

Copy link
Copy Markdown
Contributor

Thank you @o-sama ! LGTM.

@liaodaniel - could you please look at these breaking API changes since you are the original author of this section of code and make sure you are happy with them?

Yep, this looks good from my end. Part of me felt that I probably should have waited a bit until the feature was out of beta before introducing it but the feature seemed too compelling to wait 😄

@liaodanielliaodaniel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @liaodaniel !
Merging.

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Jul 18, 2023
@gmlewis
gmlewis merged commit 6c1dedd into google:masterJul 18, 2023
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API ChangePR 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.

3 participants

@o-sama@liaodaniel@gmlewis