JSDoc Migration

[mtraynham]

Second try, things went a little haywire with that rebase (accidentally had some changes stashed...)

A WIP PR for the JSDoc migration. It uses jsdoc-to-markdown, which unlike the standard jsdoc suite, only generates a single file. I went ahead and migrated the core, bar-chart and part of the base-mixin as a quickstart.

I added a grunt watch task to easily see the differences between each save.

Example output: https://github.com/mtraynham/dc.js/blob/jsdoc_migration/web/docs/api-latest.md

May need to revise some of the tags, JSDoc likes the use of classes, so I'm still trying to understand what all is available.

[gordonwoodhull]

It looks like you still have some merge problems in your output: look at the Bar Chart heading.

The output is beautiful, and I'm glad that it produces a single page output, which is nicer for exploration IMO.

I love how the output is dense and mostly free of redundancy. I get frustrated trying to find the actual content in many generated docs, versus all the boilerplate. In fact the only repetitive, redundant thing is the "static property of dc" but that is nothing.

It's also great how it's all cross-linked, and the appearing anchors on the function names when tapping on them on a tablet are a life saver for me... or rather for the people I help when lying around reading SO.

[gordonwoodhull]

BTW I think when a PR goes bad you can just force push the branch and it will reset, since it's nothing more than a comparison between branches with some comments stuck on it.

[mtraynham]

Thanks for the feedback. The amount of documentation is a bit overwhelming, so if you see anything wrong, let me know.

In alphabetical order, I've changed everything up to color-mixin.js.

[gordonwoodhull]

Wow, awesome. I'll be glad to take half if you get sick of it. It must be tedious work!

[mtraynham]

😄 Feel free to, I was debating if a script would help, but I think I'd spend more time figuring that out than just copy-pasting the docs.

Also, on the topic of look and feel, I was looking into that jsdoc-to-markdown library. It has a ton of a features, but the one in particular is https://github.com/jsdoc2md/dmd#customising. If we don't like something, we can just create a partial handlebar template to make it look different.

[mtraynham]

Alrighty, all of them migrated. Honestly, it might be nice to have both versions of the API docs available (jsdoc & jsdoc-to-markdown).

[gordonwoodhull]

Holy cow. That's amazing. Thank you!!!

I agree, it might be nice to have straight HTML docs, as well as markdown.

Having real docs makes me think about having a real site, like this one. Our very capable technical writer used Jeckyll for that and I think it came out really nice.

Right now we've got so much front matter on the dc.js site that you have to page down to see the demo.

[mtraynham]

😄 Agreed an html documentation page would be nice off the ghpage.

So I have a few left-overs here that would ultimately call this done or are nice to haves.

1. A review of these docs would be good.
2. Add any changes to the layout/generation of jsdoc-to-markdown.
3. HTML docs (& styling)
4. Custom tags (@mandatory/@required)

[gordonwoodhull]

@mtraynham, I'm ready to merge this for beta 18 but I'm getting a ton of errors.

I think grunt-changed was masking them (because they succeed with jshint but not jscs).

With your branch I get 110 errors mostly like this:

Invalid JSDoc @param argument name at src/coordinate-grid-mixin.js :
   162 |     * @memberOf dc.coordinateGridMixin
   163 |     * @instance
   164 |     * @param {Boolean} [zoomOutRestrict]
---------------^
   165 |     * @returns {Chart}
   166 |     */
Invalid JSDoc @param argument name at src/coordinate-grid-mixin.js :
   197 |     * @memberOf dc.coordinateGridMixin
   198 |     * @instance
   199 |     * @param {svg} [gElement]
---------------^
   200 |     * @returns {Chart}
   201 |     */

which is apparently jscs-dev/node-jscs#778, validateJSDoc deprecated.

With #975 updated dependencies, I instead get 40-50 of

unavailable tag instance at src/base-mixin.js :
   866 |     * @name filters
   867 |     * @memberOf dc.baseMixin
   868 |     * @instance
---------------^
   869 |     * @return {Array<*>}
   870 |     */
unavailable tag instance at src/base-mixin.js :
   893 |     * @name onClick
   894 |     * @memberOf dc.baseMixin
   895 |     * @instance
---------------^
   896 |     * @param {*} datum
   897 |     */

Both report some valid errors as well. Definitely will work through this over the afternoon, but I wanted to check with you in case you have some clues.

[mtraynham]

Hey Gordon,
Should have pointed that out, since I never tried merging the two branches. The old JSCS rules will validate the jsdoc param name against the function that they are tagged against... which is a bit of a nuisance because it required me changing all the _ params to actual param names. I was trying to avoid changing actual code for this PR.

The newer version of JSCS should complain as well... which is kind of strange that you have less errors after the merge. With JSCS, we've been using the Google preset, which basically takes our own rules and overlays them. So, if you wish to override any of the jsdoc rules, they can all be found here:
https://github.com/jscs-dev/node-jscs/blob/master/presets/google.json#L79

It seems like @instance is a proper tag according to them, although, it's rule is instance: false, which makes me think they error this case.
https://github.com/jscs-dev/jscs-jsdoc/blob/master/lib/tags/jsdoc3.json#L40

Seems to be documented according to the jsdoc spec though... http://usejsdoc.org/tags-instance.html

So, my suggestion may be to just either turn off that checkParamNames rule or fix the param names.

The @instance case, delegates the doc output you didn't particular care for:

static property of dc
instance property of dc.baseMixin

So I leave it up to you, if you just want to drop them or figure out why they error with JSCS.

[gordonwoodhull]

Cool, I'm fine with fixing those parameter names. I like descriptive parameter names.

And I'll figure what to do about @instance.

It was pretty much all "I don't like JSDoc" with the old dependencies and all "I don't like @instance" with the new.

BTW, how did you review the output? I tried Harp, but it comes out pretty raw without the github/jekyll processing. Maybe I'll just test this in a branch.

[gordonwoodhull]

Okay, looks like we're using a mixture of jsdoc3 and closurecompiler jsdoc annotations

So the correct setting is "checkAnnotations": true

Now I attack those parameter names.

[mtraynham]

Ahh good catch! To review changes locally, I was using Chrome and MarkView. There may be better Markdown extensions, but I haven't tried any of them. If you plan on using MarkView, sometimes these browsers restrict the extensions from executing on local files. You can enable that in the extensions settings for it:

Allow access to file URLs

[gordonwoodhull]

MarkView: Nice.

Oh boy, on the parameter names, it's not less errors, it's just quitting after 50... Guess I'll put that off.

I'm also not sure how to review this except by reading the entire documentation all over again, since all of the documentation shows as changed. I'm pushing this back to the beta 18 branch again for now:

https://github.com/dc-js/dc.js/tree/hotfix/2.0.0-beta.18

[gordonwoodhull]

It looks like the default values of properties are mostly missing.

[mtraynham]

I might have missed a few. The kind of strange thing is the default param value is not the same as default value, but I treated it that way.

[gordonwoodhull]

Yeah, I noticed that too. Weird. 😄

I'm going ahead with it and will search the old docs for defaults later. It's important information.

Docs look amazing, so glad to publish this.

[mtraynham]

Awesome! Thanks @gordonwoodhull ! Also, for whatever discrepancies, should we create a check-list issue? Maybe a new PR?

[gordonwoodhull]

I think it's just as easy to fix them as to list them, but it just means a bunch of hours of staring at the old and new docs in parallel, and that takes some concentration.

Certainly PRs are welcome, from you or anyone else who wants to look at this. Likely there are defaults that were wrong in the old docs...