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-language inline docs page #7354

Merged
merged 10 commits into from May 25, 2024

Conversation

vikram-dagger
Copy link
Contributor

No description provided.

@vikram-dagger
Copy link
Contributor Author

I had some difficulty with this page, as the Python examples were different from the others. I have tried to make them all consistent now, but even so differences persist (see inline comments) and the snippets should be reviewed again.

Flagging this for re-review for each code snippet @kpenfound @helderco @TomChv

```python file=./snippets/documentation/python/object.py
```

Here is an example of the result from `dagger call --help`:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Python output differs from that of the Go and TypeScript ones.

  • For Go and TypeScript, the inline comments for object/struct properties appear in the output of dagger functions but not in the flag list of dagger call --help
  • For Python, the inline comments appear as flags in dagger call --help but not in the output of dagger functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing there is a fix needed to my code snippets, but I'm not sure what it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Those are different things. dagger functions only show the available functions from selected object. It doesn't show arguments and that includes the constructor of course. It also doesn't show the description of the object that you're currently viewing, but dagger call --help should. So stick to --help as it shows all three:

  • Main object's name and description and constructor arguments
  • Currently selected function and description and available arguments
  • Available functions to chain

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing there is a fix needed to my code snippets, but I'm not sure what it is.

I'll comment inline what I can notice just by the diff for now.

@vikram-dagger vikram-dagger force-pushed the docs-238-inline-doc branch 2 times, most recently from 15c1083 to 1d0f621 Compare May 14, 2024 14:59
@helderco
Copy link
Contributor

Deploy preview failed, so I'm rebasing from the web to get it built.

@helderco
Copy link
Contributor

Ok, should have checked the error. Looks like reference docs are failing.

@helderco
Copy link
Contributor

@vikram-dagger, let me know when the preview is fixed. And to ask you to update the outputs from --help based on latest dagger version. 🙏

Age int
}

func New(name string, age int) *MyModule {
Copy link
Contributor

@helderco helderco May 15, 2024

Choose a reason for hiding this comment

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

The constructor's arguments aren't documented so their flags won't show the descriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This didn't work. Am I doing something wrong?

package main

// The struct represents a single user of the system.
type MyModule struct {
	// The name of the user.
	Name string
	// The age of the user.
	Age int
}

func New(name string, age int) *MyModule {
	return &MyModule{
		// The name of the user.
		Name: name,
		// The age of the user.
		Age: age,
	}
}
FUNCTIONS
  age           The age of the user.
  name          The name of the user.

ARGUMENTS
      --age int       [required]
      --name string   [required]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@field()
age: number

constructor(age: number, name: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Constructor arguments here also not documented, that's why their flags don't show the descriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This also didn't work with the code below

import { object, field, func } from "@dagger.io/dagger"

/**
 * The object represents a single user of the system.
 */
@object()
class MyModule {
  /**
   * The name of the user.
   */
  @field()
  name: string

  /**
   * The age of the user.
   */
  @field()
  age: number

  constructor(age: number, name: string) {
    // The name of the user.
    this.name = name
    // The age of the user.
    this.age = age
  }
}
FUNCTIONS
  age           The age of the user.
  name          The name of the user.

ARGUMENTS
      --age int       [required]
      --name string   [required]

Copy link
Member

Choose a reason for hiding this comment

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

You need to document the code following JsDoc convention with /** **/ comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@vikram-dagger vikram-dagger force-pushed the docs-238-inline-doc branch 2 times, most recently from a615fa8 to 7d1a014 Compare May 16, 2024 11:30
@vikram-dagger
Copy link
Contributor Author

@vikram-dagger, let me know when the preview is fixed. And to ask you to update the outputs from --help based on latest dagger version. 🙏

Preview is fixed. --help output fixed for first example. Still not able to get the second one to display the descriptions properly, see my inline replies with code and output.

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.

I reviewed the TS code and answered to your doc question.

The reason why we do not support // is because the TS compiler API doesn't consider them as comment.
It only include JSDoc style comment.

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 6efd184 into dagger:main May 25, 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

4 participants