-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add support for enterprise billing cost centers API #3803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3803 +/- ##
==========================================
+ Coverage 92.23% 92.27% +0.04%
==========================================
Files 191 192 +1
Lines 13823 13896 +73
==========================================
+ Hits 12750 12823 +73
Misses 884 884
Partials 189 189 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, @gjasny - thank you!
Just one minor tweak, please, then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29
stevehipwell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
gmlewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @gjasny and @stevehipwell!
LGTM.
Merging.
| } | ||
|
|
||
| // CostCenterListOptions specifies optional parameters to the EnterpriseService.ListCostCenters method. | ||
| type CostCenterListOptions struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better:
| type CostCenterListOptions struct { | |
| type ListCostCenterOptions struct { |
It's for consistency with:
go-github/github/actions_artifacts.go
Line 59 in 943b801
| type ListArtifactsOptions struct { |
go-github/github/actions_workflow_runs.go
Line 61 in 943b801
| type ListWorkflowRunsOptions struct { |
go-github/github/actions_runner_groups.go
Line 74 in 943b801
| type ListOrgRunnerGroupOptions struct { |
go-github/github/actions_workflow_jobs.go
Line 60 in 943b801
| type ListWorkflowJobsOptions struct { |
go-github/github/codespaces.go
Line 118 in 943b801
| type ListCodespacesOptions struct { |
|
|
||
| // CostCenterListOptions specifies optional parameters to the EnterpriseService.ListCostCenters method. | ||
| type CostCenterListOptions struct { | ||
| State *string `url:"state,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a comment:
// Set to `active` or `deleted` to only list cost centers in a specific state.|
|
||
| // CostCenterRequest represents a request to create or update a cost center. | ||
| type CostCenterRequest struct { | ||
| Name *string `json:"name,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameter is required whether we create or update cost center:
| Name *string `json:"name,omitempty"` | |
| Name string `json:"name"` |
|
Whups, sorry, @alexandear that I missed your review. |
Sure. See #3805 |
|
Thanks for the fast and thorough review. |
No description provided.