Skip to content

refactor: Refactor org_custom_roles.go into multiple files - #3291

Merged
gmlewis merged 2 commits into
google:masterfrom
felixlut:orgs-custom-roles-refactor
Oct 1, 2024
Merged

refactor: Refactor org_custom_roles.go into multiple files#3291
gmlewis merged 2 commits into
google:masterfrom
felixlut:orgs-custom-roles-refactor

Conversation

@felixlut

@felixlutfelixlut commented Sep 24, 2024

Copy link
Copy Markdown
Contributor

Continuation of #3281 (comment)

The /organizations/ORGANIZATION_ID/custom_roles endpoint has been deprecated in favor of /orgs/ORG/custom-repository-roles. So the orgs_custom_roles.go file should be refactored. The file also deals with Organization roles, which is a separate kind of resource to the Organization Repository Roles. For this reason this PR aims to separate the file into two:

  • orgs_custom_repository_roles.go - API docs - /orgs/ORG/custom-repository-roles
  • orgs_organization_roles.go - API docs - /orgs/ORG/organization-roles

@felixlut
felixlut marked this pull request as ready for review September 24, 2024 16:28
@gmlewisgmlewis changed the title chore: refactor org_custom_roles.go strucutre into multiple fileschore: Refactor org_custom_roles.go into multiple filesSep 25, 2024

@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, @felixlut !
Any time a new file is created, we bump the copyright year to the current year, but if a file is renamed or otherwise edited, we leave the copyright year alone...
So just a couple minor nits, please, then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

Comment threadgithub/orgs_custom_repository_roles.go Outdated
Comment threadgithub/orgs_custom_repository_roles_test.go Outdated
@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label Sep 25, 2024
@codecov

codecovBot commented Sep 25, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.97%. Comparing base (2b8c7fa) to head (d590920).
Report is 127 commits behind head on master.

Files with missing linesPatch %Lines
github/orgs_custom_repository_roles.go95.00%1 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #3291 +/- ##
==========================================
- Coverage 97.72% 92.97% -4.75% 
==========================================
Files 153 172 +19 Lines 13390 11715 -1675 ==========================================
- Hits 13085 10892 -2193 - Misses 215 729 +514 - Partials 90 94 +4 

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

Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>

@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, @felixlut !
LGTM.

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

@felixlutfelixlut changed the title chore: Refactor org_custom_roles.go into multiple filesrefactor: Refactor org_custom_roles.go into multiple filesSep 26, 2024

@tomfeigintomfeigin 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

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Oct 1, 2024
@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @tomfeigin !
Merging.

@gmlewis
gmlewis merged commit cd59fac into google:masterOct 1, 2024
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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@felixlut@gmlewis@tomfeigin