If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED WONTFIX

Status

Firefox OS
Gaia::SMS
RESOLVED WONTFIX
3 years ago
7 months ago

People

(Reporter: rishav_, Unassigned, Mentored)

Tracking

({meta})

Firefox Tracking Flags

(Not tracked)

Details

Comment hidden (empty)
(Reporter)

Updated

3 years ago
Assignee: nobody → rishav006
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/
(Reporter)

Updated

2 years ago
Assignee: rishav006 → nobody
Hey Oleg, this seems something light, could I help?
Flags: needinfo?(azasypkin)
(Reporter)

Comment 3

2 years ago
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
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?
(Reporter)

Comment 5

2 years ago
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
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!
Flags: needinfo?(azasypkin)
(Reporter)

Comment 7

2 years ago
Yeah.. It will make reviewing easy too :)
Yeah I agree with Oleg. Where could I start?
Flags: needinfo?(rishav006)
Flags: needinfo?(azasypkin)
(Reporter)

Comment 9

2 years ago
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.
Flags: needinfo?(rishav006)
Keywords: meta
(Reporter)

Updated

2 years ago
Depends on: 1223462
(Reporter)

Updated

2 years ago
Summary: [SMS] Proper commenting for purpose of JSDOC → [Messages][Docs] Proper commenting for purpose of JSDOC
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!
Flags: needinfo?(azasypkin)
(Reporter)

Comment 11

2 years ago
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
Flags: needinfo?(kalpeshk2011)
Flags: needinfo?(azasypkin)
(Reporter)

Comment 12

2 years ago
I would suggest to go for views
- Conversation/ 
- Inbox/

these two for now. (there is several comments already in the code)
Rishav,
yeah sure I will file a bug once you and Oleg choose an appropriate file
Flags: needinfo?(kalpeshk2011)
(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!
Flags: needinfo?(azasypkin)
(Reporter)

Comment 15

2 years ago
Thanks Oleg for giving tip for running JSDOC locally.
(Reporter)

Updated

2 years ago
Depends on: 1241175
Mass closing of Gaia::SMS bugs. End of an era :(
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → WONTFIX
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in before you can comment on or make changes to this bug.