1152408 - [Messages][Docs] Proper commenting for purpose of JSDOC

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::SMS, defect)

Comment 1

[Oleg Zasypkin [:azasypkin]]

Good sources of JSDoc rules:

1. https://developers.google.com/closure/compiler/docs/js-for-compiler;

2. https://code.google.com/p/jsdoc-toolkit/wiki/TagReference;

3. http://usejsdoc.org/

Comment 2

[Kalpesh Krishna [:martianwars]]

Hey Oleg, this seems something light, could I help?

Comment 3

[kumar rishav (:rishav_)]

Hey Kalpesh,
Yeah sure, if you want to go for this. 
But this need proper understanding of method what it is doing so that you can give better comments, so that it will be helpful for contributor to understand the code base.
You can find us on irc, if you stuck somewhere :)

BTW we(contributors n developers of sms app) agreed on to write comments when anyone work on any method in his patch, then write comment for same too. (i.e which method you touch, it's nice if you write jsdoc style comment for that method).

Thanks

Comment 4

[Kalpesh Krishna [:martianwars]]

Well I am familiar with PHPDocumentor and JSDoc is similar if I'm not wrong?
So this isn't really an issue in a particular file right?

Comment 5

[kumar rishav (:rishav_)]

Oh, I didn't mean that about the JSdoc format :) 

What i meant about the comment (i.e info regarding the method etc).

For example, MessageManager.markThreadRead()
So, 
you should know what this method does, what argument(&type) it takes, what it return etc.
It won't be difficult, once you read the code, you will understand it :)
Also it will help you to understand the sms codebase :)

Hope you got me

Thanks

Comment 6

[Oleg Zasypkin [:azasypkin]]

In addition to what :rishav said - yeah it's more like a meta bug, but I think we can create separate bugs to have proper comments for particular file, like "Improve JSDoc comments in shared/selection_handler.js" or "Improve JSDoc comments in shared/contacts.js". And add them as dependencies for this bug.

Happy to clarify everything that is not clear!

Comment 7

[kumar rishav (:rishav_)]

Yeah.. It will make reviewing easy too :)

Comment 8

[Kalpesh Krishna [:martianwars]]

Yeah I agree with Oleg. Where could I start?

Comment 9

[kumar rishav (:rishav_)]

So, I would suggest to start with service folder (SMS/services/) and if u ask me which file.. Then I would say..start with MessageManager object (message_manager.js). Make this bug as meta, and file a new bug for it.

Comment 10

[Oleg Zasypkin [:azasypkin]]

Yeah, MessageManager looks like a good candidate, I've also left comment in bug 1223462 about few more files that are very connected to MessagesManager and could be updated altogether.

Thanks!

Comment 11

[kumar rishav (:rishav_)]

Hey Kalpesh,
It will be nice if you file/open other bugs for this meta bug 1152408. 

Oleg, can you help kalpesh in grouping another set of files for next JSDOC bug. 


Thanks

Comment 12

[kumar rishav (:rishav_)]

I would suggest to go for views
- Conversation/ 
- Inbox/

these two for now. (there is several comments already in the code)

Comment 13

[Kalpesh Krishna [:martianwars]]

Rishav,
yeah sure I will file a bug once you and Oleg choose an appropriate file

Comment 14

[Oleg Zasypkin [:azasypkin]]

(In reply to Kalpesh Krishna [:martianwars] from comment #13)
> Rishav,
> yeah sure I will file a bug once you and Oleg choose an appropriate file

Will agree with :rishav here - views/inbox/js/inbox.js - is a good candidate. Conversation view is going to change and be split, so it maybe less attractive to you :) So if you'd like work on this - just file a bug and outline the scope of the work there!

Also today we've learned that we can test jsdoc locally to see how your changes look like:

* Run "make docs" (to generate docs for all apps) or "gulp jsdoc:sms" (to generate docs for sms app only);

* Generated docs will be placed to ./docs/ folder - so you can run http-server (e.g [1]) there and open documentation in browser!

[1] https://www.npmjs.com/package/http-server

Thanks!

Comment 15

[kumar rishav (:rishav_)]

Thanks Oleg for giving tip for running JSDOC locally.

Comment 16

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 17

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(