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
docs: Revise quickstart with code-first approach #7326
Conversation
4d9f4f2
to
a62d5fa
Compare
|
||
## Requirements | ||
|
||
To use this tutorial, you only need the Dagger CLI installed on your machine. | ||
This quickstart will take you approximately 30 minutes to complete. |
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.
The target was 10 min. However there is a lot to cover, so I couldn't get it down to under this time limit. For the moment, I'm being explicit about the time required. I'm open to ideas to condense/cut some material.
- The [Node module's `install()` function](https://github.com/dagger/dagger/blob/9e59bae142f64975b7c9ad851e6bd4901d43513a/sdk/typescript/dev/node/src/index.ts#L109) uses the `container().withExec()` method to define the command to download dependencies in the container (`npm install`). | ||
- The [Node module's `commands().run()`](https://github.com/dagger/dagger/blob/9e59bae142f64975b7c9ad851e6bd4901d43513a/sdk/typescript/dev/node/src/commands.ts#L13) function accepts an array of command line arguments and readies them for execution in the container, also using the `container().withExec()` method. | ||
|
||
Using Dagger Functions from the Node module instead of the core Dagger API offers some advantages: |
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.
IMO this is not a very strong example or reason to use a Dagger module. It has almost the same lines of code as the previous version. I'm looking for ideas to make this example better or show value from refactoring with the module. This could also be a prompt to improve the module itself and add more functions to it.
Ping @TomChv for thoughts as well.
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.
Just 1 required change with the exports
.with_exec(["npm", "install"]) | ||
.with_exec(["npm", "run", "build"]) | ||
.directory("./dist") | ||
.export("./build") |
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.
These sections are incorrect, we cannot export from within a function
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.
Fixed
c6d3cd8
to
168d739
Compare
Ping @kpenfound for second review |
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.
Looks great! Really like the new flow
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 went through this guide today and I was surprised by a few things.
Based on our most recent discussion earlier this week, I was expecting things to look something like this:
- Under 10 minute quickstart guide where we dive right into the code and daggerize a sample project with a realistic looking pipeline like build, lint ,test.
- A longer tutorial that is 30-60 minutes long that builds upon all of the concepts introduced in the quickstart, refactors some of the things we did "the hard way" and provides a lot more context about more advanced dagger topics
I like the overall code first approach here, but the current quickstart feels way too verbose and complex to me. I walked through it reading the whole thing and it took me 8 minutes of reading what felt like way too "markety" "what is dagger?" explanation before I got to the part where I install the CLI.
I'd really love to see the quickstart start on this page: /quickstart/setup and move all the other pre-amble to a different section.
Same goes for quickstart/cli , there should be a place where we talk about all of the different options for installing the CLI, but for the purpose of the quickstart it should be a one-liner using homebrew or curl if we wanted to be more platform agnostic.
I have a lot more specific comments about individual parts of the quickstart but it would be great to get aligned on what we expect the shape of this to be first.
4017cd1
to
cb5bc53
Compare
@levlaz thank you for the review and feedback!
This might be a misunderstanding. AFAIK the plan was a single quickstart. Having two separate ones might be confusing and we also don't have a place in our docs structure to put the longer "tutorial" version you refer to. There is a "Guides" section but there is a possibility that page will be removed in favour of simple "Integrations" pages
I agree with you on length, see my note in #7326 (comment).
Thank you for the suggestions! I've updated the PR to move the text-heavy introduction to the main docs index page and only linked to it from the quickstart, so that should (presumably) cut 8 minutes off the top. Please lmk what you think.
I would not be in favour of this change. There is value in having a single canonical set of install instructions that is used everywhere. It also makes it much easier to maintain, because we only need to update a single common page for any changes. |
@levlaz I found a way to do this within the structure of the current quickstart. Try the new speedrun, let me know if you like it! FYI @kpenfound |
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.
@vikram-dagger the speedrun is fantastic, I love it, it's exactly what I had in mind with my comments from yesterday and you did an excellent job succinctly showing off the core pillars of what dagger can do.
Couple of small issues.
- I think you meant to put some other command here because it says
curl localhost:80
twice, and nginx is not running by default.
...running services
curl localhost:80
- When we go through the guide with python, we don't tell the user how to install the sdk into the IDE to provide typehints which makes the experience significantly worse when you are writing code ( I don't think we talk about this in the regular quickstart either). I think its ok to leave it out of the speedrun, but we should at least reference this part of the docs once we get to this section of the quickstart https://docs.dagger.io/manuals/developer/python/742020/ide-integration
I suggest you make CLI installation a pre-requisite of the quickstart. It'll reduce the duplication there and make the tutorial leaner. |
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
b671a98
to
a786859
Compare
We discussed various changes to this quickstart, which are being implemented in #7382. I'm leaving this PR open as a draft temporarily for reference and comparison. |
Now closing in favour of #7382 |
This commit revises the quickstart to use an example application and build a pipeline for it by writing bespoke Dagger Functions.
Also see #7196 and #7275
Closes #7157