-
Notifications
You must be signed in to change notification settings - Fork 881
split examples into examples and quickstart #4634
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
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
b54b127
to
bea6c19
Compare
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.
@ericpaulsen - I like the new structure. I also see some references of examples
still in the code which makes things a bit confusing IMO. Additionally, I made a few changes
392b23f - move lima example into docs. it is not a template. i also moved scripts to the root
bea6c19 and 9c526de use the proper dir for coder templates init
in makefile
I may have incorrectly understood #4201, but for coder templates init
do we intend on showing both quickstarts AND examples? cc @kylecarbs? I assumed we just wanted to display the former.
cc @kylecarbs to review as well
@@ -27,7 +27,7 @@ import ( | |||
"github.com/coder/coder/coderd/userpassword" | |||
"github.com/coder/coder/coderd/util/slice" | |||
"github.com/coder/coder/codersdk" | |||
"github.com/coder/coder/examples" | |||
examples "github.com/coder/coder/templates" |
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.
@ericpaulsen Shouldn't this specifically be the quickstarts
folder since this is what we want to show in coder templates init
?
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.
@bpmct yeah, quickstarts should be referenced when running coder templates init
.
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 think we need to change this to only reference the quickstarts since it's used for the autoimport behavior. I might be missing something though?
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 guess, I'm asking if we should change it to this
quickstart "github.com/coder/coder/templates/quickstart"
I have read the CLA Document and I hereby sign the CLA |
@bpmct anything pending on this PR, or are we good to merge (after conflict resolution)? |
Let's wait on @kylecarbs to review. |
@ericpaulsen I think we're super close, but some more work has to be done. For example, |
@bpmct I think it's fine that |
Ah, yeah. That's correct. I was wrong |
This Pull Request is becoming stale. In order to minimize WIP, prevent merge conflicts and keep the tracker readable, I'm going close to this PR in 3 days if there isn't more activity. |
this PR addresses step 1 of #4201, and separates example templates from quickstarts.
the
quickstart
directory houses subdirectories for each type of compute, containers and virtual machines. within these subdirs are the example templates for each cloud/infra.with the help of @Emyrk, the
examples.go
test is now more generic and lists all templates in thequickstart
subdirs:alternatively the
examples
directory is intended for more specific, bespoke template examples. we can extendexamples_test.go
to include theexamples
dir if need be.i removed the
bare
template since it was not usable out-of-the-box, and thegcp-vm-container
,docker-code-server
templates because they were duplicates of existing templates. feedback appreciated.Coder templates init
New templates listed when running
templates init
. Used to list 15 templates, now lists 9