-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: Add support for remaining Codespaces APIs #3886
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
base: master
Are you sure you want to change the base?
feat: Add support for remaining Codespaces APIs #3886
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3886 +/- ##
==========================================
+ Coverage 92.48% 92.54% +0.05%
==========================================
Files 200 200
Lines 14564 14679 +115
==========================================
+ Hits 13469 13584 +115
Misses 895 895
Partials 200 200 ☔ 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.
Thank you, @Not-Dhananjay-Mishra!
…issions now returns a struct
|
@gmlewis, Thanks for the suggestions! Fixed 👍 |
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, @Not-Dhananjay-Mishra!
One minor tweak, please, then I think we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29
alexandear
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.
Left some suggestions. Feel free to ask questions.
github/codespaces.go
Outdated
| // GitHub API docs: https://docs.github.com/rest/codespaces/codespaces#update-a-codespace-for-the-authenticated-user | ||
| // | ||
| //meta:operation PATCH /user/codespaces/{codespace_name} | ||
| func (s *CodespacesService) Update(ctx context.Context, codespaceName string, opt *UpdateCodespaceOptions) (*Codespace, *Response, error) { |
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.
| func (s *CodespacesService) Update(ctx context.Context, codespaceName string, opt *UpdateCodespaceOptions) (*Codespace, *Response, error) { | |
| func (s *CodespacesService) UpdateCodespace(ctx context.Context, codespaceName string, opts *UpdateCodespaceOptions) (*Codespace, *Response, error) { |
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.
I see many method in codespace.go where method name are like Delete, Stop, Start and List. So can't we use Update only here?
Relates: #2362
This PR adds support for remaining endpoint of Codespaces