fix!: Resolve inconsistent options for create and update on custom org role

[munlicode]

BREAKING CHANGE: CreateCustomOrgRole and UpdateCustomOrgRole now take their own options.

• Split structs for CreateCustomOrgRole and UpdateCustomOrgRole into CreateOrgRoleOptions and UpdateOrgRoleOptions.
• Set name required option on create operation.
• Set all options to be optional on update operation.

Fixes: #4074.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.65%. Comparing base (16e8203) to head (29d7287).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4075   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files         210      210           
  Lines       19416    19416           
=======================================
  Hits        18184    18184           
  Misses       1034     1034           
  Partials      198      198           

☔ 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]

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

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra

[alexandear]

CreateOrgRoleOptions are body parameters, but we typically use the Options  suffix for query parameters. Also, a request body is required when creating a custom org role.
Additionally, we are returning a role, not roles.

Therefore, I propose the following renamings:

CreateOrgRoleOptions -> CreateCustomOrgRoleRequest
CustomOrgRoles -> CustomOrgRole

Suggested change

	func (s *OrganizationsService) CreateCustomOrgRole(ctx context.Context, org string, opts *CreateOrgRoleOptions) (*CustomOrgRoles, *Response, error) {
	func (s *OrganizationsService) CreateCustomOrgRole(ctx context.Context, org string, request CreateCustomOrgRoleRequest) (*CustomOrgRole, *Response, error) {

We're breaking this endpoint anyway, so let’s do it in the best possible way.

[munlicode]

Would it not be confusing to have request and req ?

func (s *OrganizationsService) CreateCustomOrgRole(ctx context.Context, org string, request CreateCustomOrgRoleRequest) (*CustomOrgRole, *Response, error) {
	u := fmt.Sprintf("orgs/%v/organization-roles", org)

	req, err := s.client.NewRequest("POST", u, request)
	...

If it is fine, I am ready to rename opts to request.

[alexandear]

We can call it Body, but that would require significant refactoring.

And the Request prefix is common across the code base:

go-github/github/repos.go

Line 2324 in 09946e5

func (s *RepositoriesService) Transfer(ctx context.Context, owner, repo string, transfer TransferRequest) (*Repository, *Response, error) {

go-github/github/orgs_network_configurations.go

Line 132 in 09946e5

func (s *OrganizationsService) CreateNetworkConfiguration(ctx context.Context, org string, createReq NetworkConfigurationRequest) (*NetworkConfiguration, *Response, error) {

go-github/github/actions_runners.go

Line 87 in 09946e5

func (s *ActionsService) GenerateRepoJITConfig(ctx context.Context, owner, repo string, request *GenerateJITConfigRequest) (*JITRunnerConfig, *Response, error) {

[alexandear]

The parameter name can be simply create instead of request.

[alexandear]

Same as above:

Suggested change

	func (s *OrganizationsService) UpdateCustomOrgRole(ctx context.Context, org string, roleID int64, opts *UpdateOrgRoleOptions) (*CustomOrgRoles, *Response, error) {
	func (s *OrganizationsService) UpdateCustomOrgRole(ctx context.Context, org string, roleID int64, request UpdateCustomOrgRoleRequest) (*CustomOrgRole, *Response, error) {