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

docs: Add multi-lang module structure page #7411

Merged
merged 10 commits into from
May 24, 2024

Conversation

vikram-dagger
Copy link
Contributor

No description provided.

@vikram-dagger vikram-dagger marked this pull request as ready for review May 19, 2024 19:17
@vikram-dagger vikram-dagger marked this pull request as draft May 20, 2024 19:35
@vikram-dagger vikram-dagger marked this pull request as ready for review May 21, 2024 13:31
@vikram-dagger
Copy link
Contributor Author

This was a difficult page to merge, because the content was significantly different for each languages. I needed to backfill some content and also adjust the structure of the sections. To confirm there are no errors, I would like review and approval from experts in our team. Pinging:

When reviewing, if you see areas where we can modify or optimize the internal page structure further, please add comments.

@vikram-dagger vikram-dagger changed the title docs: Convert module structure page docs: Add multi-lang module structure page May 21, 2024
Copy link
Member

@TomChv TomChv left a comment

Choose a reason for hiding this comment

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

Thanks for these changes, I added some update suggestions based on new changes that were introduced with #7096.

I like the overall structure, you found a nice balance between generic information and language specific.

Comment on lines 106 to 108
"devDependencies": {
"@dagger.io/dagger": "./sdk"
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This changed with #7096 in favor of

{
  "dependencies": {
    "typescript": "^5.3.2"
    "@dagger.io/dagger": "./sdk"
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added


```shell
# executed by the runtime container
npm install --package-lock-only ./sdk
Copy link
Member

Choose a reason for hiding this comment

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

This changed with #7096, now we install the SDK using yarn v4 and add the dependencies to the package.json with npm pkg set "dependencies[@dagger.io/dagger]=./sdk"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, please check if correct

</TabItem>
<TabItem value="TypeScript">

The runtime container is currently hardcoded to run in Node.js 21.3 (although this may be configurable in future).
Copy link
Member

Choose a reason for hiding this comment

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

This will change soon when we will use LTS version, see #7433


```shell
# executed by the runtime container
npm install
Copy link
Member

Choose a reason for hiding this comment

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

Since #7096 yarn install --production

Note that now, only production dependencies will be installed, not packages defined in the devDependencies field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@vikram-dagger
Copy link
Contributor Author

Thanks for these changes, I added some update suggestions based on new changes that were introduced with #7096.

I've added these changes, can you please review once again @TomChv ? Thanks!

@vikram-dagger
Copy link
Contributor Author

Ping @vito @sipsma @jedevc @helderco @TomChv for review, see #7411 (comment)

Copy link
Contributor

@kpenfound kpenfound left a comment

Choose a reason for hiding this comment

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

One change in comment

docs/current_docs/manuals/developer/module-structure.mdx Outdated Show resolved Hide resolved
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>
@vikram-dagger vikram-dagger merged commit 6d5a3de into dagger:main May 24, 2024
59 checks passed
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.

None yet

3 participants