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

Add a flag to document all exported bindings #490

Closed
wants to merge 3 commits into from

Conversation

arv
Copy link
Contributor

@arv arv commented Aug 18, 2016

This adds a boolean flag called document-exported (defaults to false)
that effectively adds an empty comment to all exported bindings that do
not already have a JSDoc comment. It also does the same for the
members of exported classes and objects.

export class C {
  method() {}
}

Both C and C#method are now part of the generated documentation.

Related to #424

@tmcw
Copy link
Member

tmcw commented Aug 18, 2016

👍 awesome, there's a lint error - I think here - that's making CircleCI quit

@arv
Copy link
Contributor Author

arv commented Aug 18, 2016

That was not the lint error... See new commit if you care.

@arv
Copy link
Contributor Author

arv commented Aug 18, 2016

Test is timing out. Maybe I've made test/bin.js too slow by adding one more test to it?

2 observations working on this:

  1. It might be better to move the code into an infer pass. However, infer passes only update existing comments. Can it work to add new comments in an infer pass?
  2. If there was a way to add options to a fixture test testing this would have been cleaner.

@arv
Copy link
Contributor Author

arv commented Aug 18, 2016

If you like #492 then I can simplify these tests to use that.

@tmcw
Copy link
Member

tmcw commented Aug 18, 2016

Yep, can you rebase on master and check that tests should pass? Still timing out, digging more.

This adds a boolean flag called `document-exported` (defaults to false)
that effectively adds an empty comment to all exported bindings that do
not already have a JSDoc comment. It also does the same for the
members of exported classes and objects.

```js
export class C {
  method() {}
}
```

Both `C` and `C#method` are now part of the generated documentation.

Related to documentationjs#424
@arv
Copy link
Contributor Author

arv commented Aug 22, 2016

@tmcw PTAL

@tmcw
Copy link
Member

tmcw commented Aug 23, 2016

@arv looks great! In terms of:

It might be better to move the code into an infer pass. However, infer passes only update existing comments. Can it work to add new comments in an infer pass?

+1 for creating an interface for extracting comments just like we have one for inference. I don't think the inference interface (ha?) makes sense for extraction, so I wrote #502 which creates an 'extractor' concept and then extracts the two kinds into their own files. It seems to work pretty well - the only weirdness is around addComments, which I inject into the extractors so we don't have to pass arguments everywhere. Thoughts? If you're into it, you can cherry-pick the commit over.

@tmcw
Copy link
Member

tmcw commented Aug 24, 2016

Merged with #502!

@tmcw tmcw closed this Aug 24, 2016
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.

2 participants