simplajs / simpla-img

An editable image you can update inline, built on Simpla
https://www.simplajs.org
MIT License
6 stars 3 forks source link

Add documentation for all JS #25

Closed bedeoverend closed 8 years ago

bedeoverend commented 8 years ago
madeleineostoja commented 8 years ago

Just found out that ESlint can lint code docs, which we should definitely do. So as you go through and comment components just add "valid-jsdoc": 2 to .eslintrc. I've been adding it below "space-infix-ops": 2,. Already added it to the generator.

By default ESlint freaks out if there's no @returns statement, and further if it doesn't have a type ({undefined} or {void} for side-effects functions). I'm still not sure which way to go with this, for example Google's Clojure compiler guidelines say if there's no return statement to omit the @returns statement, but I guess more info the better? In either case, you can turn that off if need be.

bedeoverend commented 8 years ago

Nice! My personal preference is to include a @returns statement. But, that's just my opinion, I'll add it in as I go

madeleineostoja commented 8 years ago

kewl. Yeah I prefer including @returns too, can't hurt to have too much info, and if it's missing you have to dig through the function to see if it returns anything to make sure it wasn't just left out of the docs.

bedeoverend commented 8 years ago

@seaneking do you want to look over the docs PRs, or happy for me to merge?

madeleineostoja commented 8 years ago

couple of linting errors ;-)

screen shot 2016-01-04 at 5 22 10 pm

@returns has to have a type, if it's for side-effects only make it {undefined}

And things like properties should use // Comment syntax rather than JsDoc, otherwise it'll be subject to linting (needs @returns etc.)

bedeoverend commented 8 years ago

@seaneking haha, yeah just a few. I think my linter must be broken. I'll let you know when I've patched it up

bedeoverend commented 8 years ago

@seaneking could you take a look to see if linting is passing on yours now too? I've left the /** syntax for properties in, as it's cleaner IMO and passes my linter. Maybe only cares about @returns before functions? Hopefully anyway, otherwise that's just confusing.

madeleineostoja commented 8 years ago

:+1: lgtm