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

Multi devcontainer #5076

Merged
merged 14 commits into from Apr 20, 2022
Merged

Multi devcontainer #5076

merged 14 commits into from Apr 20, 2022

Conversation

reybard
Copy link
Contributor

@reybard reybard commented Jan 21, 2022

The API endpoints that power this feature are guarded by feature flags so users noticing this flag on a release would not be at risk for harming anything, but we would not want to confuse anyone if we can avoid it. This branch should not be merged until the :codespaces_multi_devcontainer feature is fully shipped.

Fixes github/codespaces#5597 and github/codespaces#7463

This change adds the --devcontainer-path flag to the CLI to pass along to the create API:

» go run cmd/gh/main.go cs create -r getpuft/foobie -b main --devcontainer-path ".devcontainer/foobar/devcontainer.json"
reybard-getpuft-foobie-54pg696xcp5g

For the create flow it also makes use of the list-devcontainers API endpoint to provide a list of possible devcontainer.json file options for users in a prompt:

Screen Shot 2022-01-21 at 2 14 07 PM

Copy link
Member

@mislav mislav left a comment

Looks good so far! Thank you for working on this.

pkg/cmd/codespace/create.go Outdated Show resolved Hide resolved
@cli cli deleted a comment Mar 28, 2022
@reybard reybard marked this pull request as ready for review Apr 12, 2022
@reybard reybard requested review from a team as code owners Apr 12, 2022
@reybard reybard requested review from mislav (assigned from cli/code-reviewers) and removed request for a team Apr 12, 2022
@cliAutomation cliAutomation added the external label Apr 12, 2022
@cliAutomation cliAutomation added this to Needs review 🤔 in The GitHub CLI Apr 12, 2022
@reybard
Copy link
Contributor Author

reybard commented Apr 12, 2022

Friendly reminder this should not be brought into a release until https://admin.github.com/devtools/feature_flags/codespaces_multi_devcontainer is fully enabled.

@hubwriter
Copy link

hubwriter commented Apr 13, 2022

If you could let me know the date this will be included in a CLI release I'd be grateful. There's a little docs update I'll need to make. Thanks

@reybard
Copy link
Contributor Author

reybard commented Apr 13, 2022

The feature has been shipped. It is now safe to merge this PR.

mislav
mislav approved these changes Apr 19, 2022
Copy link
Member

@mislav mislav left a comment

This looks good! Only style comments here

internal/codespaces/api/api.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/create.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/create.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/create.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/create.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/create.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/create.go Show resolved Hide resolved
The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Apr 19, 2022
@reybard reybard merged commit 7678274 into trunk Apr 20, 2022
10 checks passed
@reybard reybard deleted the multi-devcontainer branch Apr 20, 2022
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Apr 20, 2022
@github-actions github-actions bot moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external
Projects
The GitHub CLI
  
Done 💤
Development

Successfully merging this pull request may close these issues.

None yet

4 participants