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

Use doc comments to define task description #103

Closed
seaneagan opened this issue Feb 26, 2015 · 12 comments
Closed

Use doc comments to define task description #103

seaneagan opened this issue Feb 26, 2015 · 12 comments
Labels

Comments

@seaneagan
Copy link
Contributor

/// Compiles stuff.
@Task()
compile() {
  // ...
}

instead of:

@Task(
  description: 'Compiles stuff.')
compile() {
  // ...
}

This would allow producing nice dartdocs for reusable task libraries!

Of course, it's not currently possible, see: http://dartbug.com/15704.

@devoncarew
Copy link
Contributor

+1!

@devoncarew
Copy link
Contributor

@seaneagan, @bwilkerson

I'm finding the new task format a little hard to read:

@Task(
    depends: const ['init'],
    description: 'Compile stuff.')
compile(GrinderContext context) {
  context.log("Compiling stuff...");
}

But, I very much like the move towards annotations in general. I think a more-easily-read format would be something like:

/// Compile stuff.
@Task(depends: init)
compile(GrinderContext context) {
  context.log("Compiling stuff...");
}

or

/// Compile stuff.
@task
@depends(init)
compile(GrinderContext context) {
  context.log("Compiling stuff...");
}

But those re-worked formats require us having the ability to pull dartdoc out of our code at runtime. Brian, what's the feasibility of using the analyzer to do this? Given a path to a library file and a symbol name (compile), could we extract the dartdoc for that symbol reliably and performantly?

@bwilkerson
Copy link

Given a path to a library file and a symbol name (compile), could we extract the dartdoc for that symbol reliably and performantly?

If the name is defined in the library file (rather than a part), then the only thing you need to do is read, scan, and parse the file, then find the symbol in the file. If it might be in a part, then you might have to repeat those steps after resolving the part URI's. That should be reasonably performant.

@seaneagan
Copy link
Contributor Author

That'd be awesome if we can use the analyzer.

Regarding splitting into multiple annotations e.g. @task and @depends, I agree it looks pretty nice. However, I'm concerned about losing auto-complete when typing @Task(. But if we did go with it, and using the analyzer doesn't work for some reason, this isn't too bad:

@task('Compile stuff.')
@depends(init)
compile(GrinderContext context) {
  context.log("Compiling stuff...");
}

but then it would have to be @task() instead of @task until dart allows that.

@devoncarew
Copy link
Contributor

Ah, so use @task(...) as both the task marker and the annotation for documentation. I like how it reads.

@devoncarew
Copy link
Contributor

Here's a blizzard of different styles:

https://gist.github.com/devoncarew/459faa419d87ef850b82

I really like this style:

@task('Compile stuff.')
@depends(init, foo, bar)
compile() {
  context.log("Compiling stuff...");
}

Shown in lower case annotations. Is the consensus that they should be uppercase? Or, only lowercase if they're constants and not constructors?

@bwilkerson
Copy link

The standard in the core library appears to be to follow normal case conventions: upper case for constructors and lower case for constants.

@seaneagan
Copy link
Contributor Author

My vote:

/// Compile stuff.
@Task
@Depends(init)
compile() {
  ...
}

or keep annotations on the same line:

/// Compile stuff.
@Task @Depends(init)
compile() {
  ...
}

The style guide says:

DO use UpperCamelCase names for classes intended to be used in metadata annotations.
If the annotation takes no parameters, you might want to create a lowerCamelCase constant for it.

So that would mean @Depends(...) and @task or @Task(...), but in order to work around http://dartbug.com/13582 I like doing just @Task as an opaque constant for now, and if that bug ever gets resolved, then we can still change it to a class without it being a breaking change to follow the style guide, and allow adding any needed optional params.

@seaneagan
Copy link
Contributor Author

I'd be fine with lowercase too, I just think we should lobby to have the style guide changed in that case.

@devoncarew
Copy link
Contributor

Let's keep it as is for now:

@Task('Compile stuff')
@Depends(init)

and keep the dartdoc option on the table for some future iteration:

/// Compile stuff.
@Task
@Depends(init)

@seaneagan
Copy link
Contributor Author

sgtm, we could do it as a non-breaking change in the future by allowing dartdoc in addition to the current method.

@devoncarew
Copy link
Contributor

Closing this for now; the documentation in task annotations works pretty well. We can always revisit this in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants