Skip to content
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

Add basic codespaces api #2722

Merged
merged 7 commits into from
Jun 16, 2023
Merged

Add basic codespaces api #2722

merged 7 commits into from
Jun 16, 2023

Conversation

asklar
Copy link
Contributor

@asklar asklar commented May 30, 2023

Resolves #2721


Behavior

Before the change?

  • no codespaces api :)

After the change?

  • adds basic Codespaces API:
    • GetAll()
    • Get(name)
    • GetForRepository(owner, repo)
    • Start(name)
    • Stop(name)

Other information


Additional info

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes (Please add the Type: Breaking change label)
  • No

If Yes, what's the impact:

  • N/A

Pull request type

Please add the corresponding label for change this PR introduces:

  • Bugfix: Type: Bug
  • Feature/model/API additions: Type: Feature
  • Updates to docs or samples: Type: Documentation
  • Dependencies/code cleanup: Type: Maintenance

@asklar
Copy link
Contributor Author

asklar commented May 31, 2023

@kfcampbell @nickfloyd could you folks take a look? Thanks!

@kfcampbell
Copy link
Member

@asklar I've approved the Actions workflow, which should get you some information about the test error.

@asklar
Copy link
Contributor Author

asklar commented Jun 1, 2023

thanks @kfcampbell ! I had forgotten to make the client type public :) (it was marked internal)
Could you kick the pipeline again?

@asklar
Copy link
Contributor Author

asklar commented Jun 2, 2023

it's failing because the Observable client doesn't expose these. @kfcampbell Is there a way to say "sorry, not supported at the moment through the reactive wrapper, use the other one"?

@asklar
Copy link
Contributor Author

asklar commented Jun 2, 2023

@kfcampbell I've pushed an empty IObservableCodespacesClient, hopefully this works now : )

@asklar
Copy link
Contributor Author

asklar commented Jun 6, 2023

conventions tests was failing, I've fixed it now. Sorry for the back and forth :) @kfcampbell

@asklar
Copy link
Contributor Author

asklar commented Jun 7, 2023

another one

@asklar
Copy link
Contributor Author

asklar commented Jun 7, 2023

@kfcampbell @nickfloyd sorry to bug you, could you kick the ci?

Copy link
Contributor

@nickfloyd nickfloyd left a comment

Choose a reason for hiding this comment

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

Thanks for doing this @asklar! ❤️ LGTM!

@nickfloyd nickfloyd merged commit da5c4d7 into octokit:main Jun 16, 2023
@asklar asklar deleted the codespacesApi branch June 16, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[FEAT]: Support Codespace APIs
3 participants