Skip to content

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

Merged
merged 47 commits into from
Mar 22, 2022
Merged

feat: Add templates to create working release #422

merged 47 commits into from
Mar 22, 2022

Conversation

kylecarbs
Copy link
Member

@kylecarbs kylecarbs commented Mar 12, 2022

This enables users to run coder start --dev to start an instance, expose it to the internet, and use a premade template for ease.

@codecov
Copy link

codecov bot commented Mar 12, 2022

Codecov Report

Merging #422 (aa267e6) into main (2818b3c) will decrease coverage by 1.06%.
The diff coverage is 49.70%.

@@            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     
Flag Coverage Δ
unittest-go-macos-latest 56.45% <44.07%> (?)
unittest-go-ubuntu-latest 60.72% <48.97%> (?)
unittest-go-windows-2022 56.12% <43.96%> (?)
unittest-js 63.19% <ø> (ø)
Impacted Files Coverage Δ
agent/usershell/usershell_windows.go 0.00% <0.00%> (ø)
cli/cliui/parameter.go 0.00% <0.00%> (ø)
cli/workspaceagent.go 15.78% <0.00%> (ø)
provisionersdk/agent.go 100.00% <ø> (ø)
site/embed_slim.go 100.00% <ø> (ø)
cli/projectupdate.go 7.24% <3.12%> (ø)
cli/workspacecreate.go 6.66% <5.40%> (ø)
cli/ssh.go 6.45% <6.45%> (ø)
cli/workspaceupdate.go 7.89% <7.89%> (ø)
cli/parameterlist.go 8.33% <8.33%> (ø)
... and 146 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2818b3c...aa267e6. Read the comment docs.

Comment on lines +31 to +34
*.tfstate
*.tfplan
*.lock.hcl
.terraform/
Copy link
Contributor

Choose a reason for hiding this comment

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

Good call 👍

Comment on lines 48 to 49

// coder login
Copy link
Contributor

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?


switch msg := msg.(type) {
case tea.WindowSizeMsg:
// topGap, rightGap, bottomGap, leftGap := appStyle.GetPadding()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not needed anymore?

import (
"github.com/charmbracelet/bubbles/key"
"github.com/charmbracelet/bubbles/list"
tea "github.com/charmbracelet/bubbletea"
Copy link
Contributor

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",
Copy link
Contributor

Choose a reason for hiding this comment

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

Exciting! 🎉

Comment on lines +36 to +44
// 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():
Copy link
Contributor

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?

Description: "Something...",
}, {
Title: "Wow, here's another!",
Description: "Another exciting description!",
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup!

Comment on lines +40 to +41
// Taken from:
// https://github.com/cloudflare/cloudflared/blob/22cd8ceb8cf279afc1c412ae7f98308ffcfdd298/cmd/cloudflared/tunnel/quick_tunnel.go#L38
Copy link
Contributor

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{})
Copy link
Contributor

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)

{
"ID": "gcp-linux",
"Name": "Develop in Linux on Google Cloud",
"Description": "Get started with Linux development on Google Cloud.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Neato!

Comment on lines 18 to 20
//go:embed */*.json
//go:embed */*.tf
files embed.FS
Copy link
Contributor

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!

Copy link
Contributor

@bryphe-coder bryphe-coder left a 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 🎉

@bryphe-coder
Copy link
Contributor

bryphe-coder commented Mar 14, 2022

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 prompt pkg usage:

=== Failed
=== FAIL: cli/cliui TestPrompt/Confirm (unknown)
panic: cannot create context from nil parent

goroutine 38 [running]:
context.WithCancel({0x0, 0x0})
	/opt/hostedtoolcache/go/1.17.7/x64/src/context/context.go:234 +0x1a5
github.com/coder/coder/cli/cliui.Prompt(0xc0000bfc80, {{0x15fff19, 0x7}, {0x0, 0x0}, 0x0, 0x0, 0x0, 0x0, 0x0})
	/home/runner/work/coder/coder/cli/cliui/prompt.go:34 +0x3c5
github.com/coder/coder/cli/cliui_test.prompt(0xc0002e9700, {{0x15fff19, 0x7}, {0x0, 0x0}, 0x0, 0x0, 0x0, 0x0, 0x0})
	/home/runner/work/coder/coder/cli/cliui/prompt_test.go:53 +0x188
github.com/coder/coder/cli/cliui_test.TestPrompt.func1.1()
	/home/runner/work/coder/coder/cli/cliui/prompt_test.go:20 +0xcd
created by github.com/coder/coder/cli/cliui_test.TestPrompt.func1
	/home/runner/work/coder/coder/cli/cliui/prompt_test.go:19 +0x15d

=== FAIL: cli/cliui TestPrompt (unknown)
panic: cannot create context from nil parent

goroutine 38 [running]:
context.WithCancel({0x0, 0x0})
	/opt/hostedtoolcache/go/1.17.7/x64/src/context/context.go:234 +0x1a5
github.com/coder/coder/cli/cliui.Prompt(0xc0000bfc80, {{0x15fff19, 0x7}, {0x0, 0x0}, 0x0, 0x0, 0x0, 0x0, 0x0})
	/home/runner/work/coder/coder/cli/cliui/prompt.go:34 +0x3c5
github.com/coder/coder/cli/cliui_test.prompt(0xc0002e9700, {{0x15fff19, 0x7}, {0x0, 0x0}, 0x0, 0x0, 0x0, 0x0, 0x0})
	/home/runner/work/coder/coder/cli/cliui/prompt_test.go:53 +0x188
github.com/coder/coder/cli/cliui_test.TestPrompt.func1.1()
	/home/runner/work/coder/coder/cli/cliui/prompt_test.go:20 +0xcd
created by github.com/coder/coder/cli/cliui_test.TestPrompt.func1
	/home/runner/work/coder/coder/cli/cliui/prompt_test.go:19 +0x15d

https://github.com/coder/coder/runs/5545694820?check_suite_focus=true#step:7:109

bryphe-coder added a commit that referenced this pull request Mar 16, 2022
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
@kylecarbs kylecarbs requested a review from a team as a code owner March 16, 2022 15:56
Copy link
Contributor

@greyscaled greyscaled left a 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`,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@kylecarbs kylecarbs merged commit c451f4e into main Mar 22, 2022
@kylecarbs kylecarbs deleted the workflow branch March 22, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants