-
Notifications
You must be signed in to change notification settings - Fork 887
feat: Add templates to create working release #422
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
@@ Coverage Diff @@
## main #422 +/- ##
==========================================
- Coverage 63.19% 62.13% -1.07%
==========================================
Files 79 191 +112
Lines 864 10856 +9992
Branches 85 85
==========================================
+ Hits 546 6745 +6199
- Misses 302 3375 +3073
- Partials 16 736 +720
Continue to review full report at Codecov.
|
*.tfstate | ||
*.tfplan | ||
*.lock.hcl | ||
.terraform/ |
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.
Good call 👍
cli/cliui/cliui.go
Outdated
|
||
// coder login |
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 wasn't sure what this trailing comment is for, maybe just leftover?
cli/cliui/list.go
Outdated
|
||
switch msg := msg.(type) { | ||
case tea.WindowSizeMsg: | ||
// topGap, rightGap, bottomGap, leftGap := appStyle.GetPadding() |
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 not needed anymore?
cli/cliui/list.go
Outdated
import ( | ||
"github.com/charmbracelet/bubbles/key" | ||
"github.com/charmbracelet/bubbles/list" | ||
tea "github.com/charmbracelet/bubbletea" |
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 like a neat library 👍
cli/ssh.go
Outdated
|
||
func workspaceSSH() *cobra.Command { | ||
return &cobra.Command{ | ||
Use: "ssh", |
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.
Exciting! 🎉
// probe, err := cloud.New() | ||
// if err != nil { | ||
// return xerrors.Errorf("probe cloud: %w", err) | ||
// } | ||
// if !probe.Detected { | ||
// return xerrors.Errorf("no valid authentication method found; set \"CODER_TOKEN\"") | ||
// } | ||
// switch { | ||
// case probe.GCP(): |
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.
Might be able to remove this?
cmd/cliui/main.go
Outdated
Description: "Something...", | ||
}, { | ||
Title: "Wow, here's another!", | ||
Description: "Another exciting description!", |
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 like these placeholder descriptions 😆
I guess these are just for testing out the API?
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.
Yup!
// Taken from: | ||
// https://github.com/cloudflare/cloudflared/blob/22cd8ceb8cf279afc1c412ae7f98308ffcfdd298/cmd/cloudflared/tunnel/quick_tunnel.go#L38 |
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.
Thanks for this context
@@ -19,15 +18,15 @@ func TestFirstUser(t *testing.T) { | |||
t.Run("BadRequest", func(t *testing.T) { | |||
t.Parallel() | |||
client := coderdtest.New(t, nil) | |||
_, err := client.CreateFirstUser(context.Background(), coderd.CreateFirstUserRequest{}) | |||
_, err := client.CreateFirstUser(context.Background(), codersdk.CreateFirstUserRequest{}) |
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 realized one thing that would make this more incremental & easier to review is to have the coderd
-> codersdk
name change as a separate PR (but I know its hard to do when a lot is changing)
template/gcp-linux/gcp-linux.json
Outdated
{ | ||
"ID": "gcp-linux", | ||
"Name": "Develop in Linux on Google Cloud", | ||
"Description": "Get started with Linux development on Google Cloud.", |
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.
Neato!
template/template.go
Outdated
//go:embed */*.json | ||
//go:embed */*.tf | ||
files embed.FS |
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 like this strategy of embedding some pre-set templates to start!
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 skimmed this because it was pretty large - but looks good to me at a high-level. The tunnel + templates stuff is exciting and seems like it'll make really easy to get started with v2 🎉
Was looking at the latest failure to see if it was one captured by some others we're seeing recently:
But this looks like a new one, related to the
https://github.com/coder/coder/runs/5545694820?check_suite_focus=true#step:7:109 |
This adds a couple of missing pieces to the docs: - More info about `./develop.sh` - Steps to run the built coder binary manually - Starting the server - Logging in - Creating a project I attempted to keep them relatively agnostic so they wouldn't be out of date immediately when #422 is merged
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.
site
changes look great to me!
@@ -17,7 +17,7 @@ const config: PlaywrightTestConfig = { | |||
// https://playwright.dev/docs/test-advanced#launching-a-development-web-server-during-the-tests | |||
webServer: { | |||
// Run the coder daemon directly. | |||
command: `go run ${path.join(__dirname, "../../cmd/coder/main.go")} daemon`, | |||
command: `go run -tags embed ${path.join(__dirname, "../../cmd/coder/main.go")} start`, |
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 enables users to run
coder start --dev
to start an instance, expose it to the internet, and use a premade template for ease.