Closed Bug 1173675 Opened 6 years ago Closed 6 years ago

link gaia jsdoc gh-page on per app readme

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gasolin, Assigned: gasolin)

Details

Attachments

(5 files)

46 bytes, text/x-github-pull-request
arthurcc
: review+
Details | Review
52 bytes, text/plain
mmedeiros
: review-
Details
46 bytes, text/x-github-pull-request
justindarc
: review+
Details | Review
46 bytes, text/x-github-pull-request
mmedeiros
: review-
mcav
: review+
squib
: review-
rudyl
: review+
steveck
: review+
Details | Review
46 bytes, text/x-github-pull-request
alive
: review+
Details | Review
link jsdoc such as http://mozilla-b2g.github.io/gaia/settings/ on per app readme. It will help new contributors to grasp our code base better.
The generated jsdoc is now hosting on gh-pages http://mozilla-b2g.github.io/gaia/settings/ . Add link on readme to help contributor grasp our code.
Attachment #8620839 - Flags: review?(arthur.chen)
The generated jsdoc is now hosting on gh-pages http://mozilla-b2g.github.io/gaia/calendar/ . Add link on readme to help contributor grasp our code.
Attachment #8620842 - Flags: review?(mmedeiros)
The generated jsdoc is now hosting on gh-pages http://mozilla-b2g.github.io/gaia/camera/ . Add link on readme to help contributor grasp our code.
Attachment #8620846 - Flags: review?(jdarcangelo)
Comment on attachment 8620846 [details] [review]
pull request for camera

LGTM. Thanks!
Attachment #8620846 - Flags: review?(jdarcangelo) → review+
Comment on attachment 8620839 [details] [review]
pull request for settings

r=me, thanks.
Attachment #8620839 - Flags: review?(arthur.chen) → review+
Attached file pull request for apps
The generated jsdoc is now hosting on gh-pages http://mozilla-b2g.github.io/gaia/<app_name>/ . Add link on readme to help contributor grasp our code.
Attachment #8622330 - Flags: review?(squibblyflabbetydoo)
Attachment #8622330 - Flags: review?(schung)
Attachment #8622330 - Flags: review?(rlu)
Attachment #8622330 - Flags: review?(mmedeiros)
Attachment #8622330 - Flags: review?(m)
Attachment #8622330 - Flags: review?(jrburke)
The generated jsdoc is now hosting on gh-pages http://mozilla-b2g.github.io/gaia/system/ . Add readme and jsdoc link to help contributor grasp our code.
Attachment #8622332 - Flags: review?(alegnadise+moz)
Comment on attachment 8622330 [details] [review]
pull request for apps

Looks good to me, please help take a look at the nits I commented on the pr.
Thank you.
Attachment #8622330 - Flags: review?(rlu) → review+
Attachment #8622332 - Flags: review?(alegnadise+moz) → review+
It seems a bit strange to me to put this in every subdirectory's README, rather than just in the main README...
Comment on attachment 8622330 [details] [review]
pull request for apps

I agree with :squib, seems fine just to do in the main readme instead of each app.

For email specifically: while we use some jsdoc-like comments in email, it is more to aid in the source code reading, not to really be useful as generated docs. The end result: the generated docs are not very useful, particularly since we do not use the tags for jsdoc to recognize modules.

So I would prefer for email to just not include the jsdoc mention in the readme: to me that indicates we are suggesting that it might be useful when in the current state it is not. 

Perhaps with this new jsdoc infrastructure it would be possible for use to now do comment changes in email to then make it useful, but I think it is too soon for email's readme to suggest users looking at the generated output.
Attachment #8622330 - Flags: review?(jrburke)
As james said not all app use or recommend to document with jsdoc, user will feel frustrated while checking the main README but linked to a void document.

I'd remove jsdoc section in email. And ringtone as well if :squib prefer so.
Flags: needinfo?(squibblyflabbetydoo)
Comment on attachment 8622330 [details] [review]
pull request for apps

LGTM, thanks!
Attachment #8622330 - Flags: review?(schung) → review+
Comment on attachment 8622330 [details] [review]
pull request for apps

I checked the generated docs for Ringtones, and like E-mail, they're not very good. Most of the function comments aren't getting picked up (probably because of how Ringtones defines some of its modules), so in the interest of not confusing people, I don't think we should advertise the Ringtones docs.
Flags: needinfo?(squibblyflabbetydoo)
Attachment #8622330 - Flags: review?(squibblyflabbetydoo) → review-
Ok it's totally up to module owner to decide if its proper or not to add the link on README. I'll remove Ringtones docs from the PR.
Attachment #8622330 - Flags: review?(m) → review+
Attachment #8620842 - Flags: review?(mmedeiros) → review+
:mmedeiros thanks for calendar review. could you help review clock part as well?
Flags: needinfo?(mmedeiros)
Comment on attachment 8622330 [details] [review]
pull request for apps

Clock documentation is not going to help contributors in any way. I think the same can be said about the Calendar documentation as well. JSDoc is not recognizing the modules (everything is displayed as part of the "Global" namespace) and there are very few documented methods.

I initially assumed that all the apps needed links to JSDoc, only now I realized it is optional.
Flags: needinfo?(mmedeiros)
Attachment #8622330 - Flags: review?(mmedeiros) → review-
Comment on attachment 8620842 [details]
pull request for calendar

changed my mind, JSDoc is not helping in any way and might confuse contributors. sorry about that.
Attachment #8620842 - Flags: review+ → review-
It's fine I'll leave calendar and clock jsdoc not listed in README.
merged for sms, keyboard, camera https://github.com/mozilla-b2g/gaia/commit/9ea3792b58302d15d99caa643f9472a4885b6485
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.