All great comments, and good to have a discussion of this.

Here’s why I brought it up.

In going through the bletiny sample app, there are almost no comments that 
describe what is going on, and why.  In slinky there are a few more, but they 
are still fairly sparse. For someone coming into this project new, trying to 
figure out what is being done, and why, is a bit of a challenge. There are some 
rudimentary comments that, for instance, identify “Task 1” and “Task 2” and a 
few sprinkled throughout some of the functions, but in general, it’s fairly 
devoid of comments. Sample apps are a great way for people new to the platform 
to begin to do real things. Take bletiny and extend it to do something more 
interesting, etc. But it’s harder to do that when it’s not entirely clear what 
it’s doing, and why.

In the console and shell code, there are almost none. The os code is well 
documented, it seems, which is very nice. The libs code is not documented at 
all in many places — newtmgr for instance has almost no comments as to what is 
going on. This has been especially trying for me personally as I try to add 
functionality to the console and shell. Especially as it pertains to 
interacting with newtmgr. Since none of it is documented, it’s really difficult 
to track down what’s going wrong and why.

I’m all about lowering the barriers to entry for developers and contributors, 
and it strikes me that this is an easy way to do so. Make it easy for folks to 
look at the code and see what’s going on, and why.

Of course this is just my $0.02, not adjusted for inflation since 1990, so take 
it as that. :-)

dg



> On Jul 11, 2016, at 2:01 PM, Sterling Hughes <[email protected]> wrote:
> 
> As Will points out, I’m OK if APIs are documented outside of the code itself. 
>  For the OS, I went through and added a bunch of Doxygen comments, but the 
> function calls and their variants were fairly simple.  Function calls that 
> take big enums as a parameter can be very lengthy to document with doxygen, 
> and I hate having them in the code itself.
> 
> To me, the bigger things that make it easier to learn new APIs are:
> 
> - Consistent operation: i.e. don’t have callbacks and event queues, don’t 
> call functions from multiple different contexts.
> - Fewer Function Calls: Easier to learn and remember a few functions, than 
> 100’s of them.  Easier to have sample code cover all our APIs.
> - Sample Code: I mostly learn through sample code — i.e. I want to do 
> something, how do I do it.  I have no problem reading relatively complex 
> source code once I’ve got the basics up & running.  But there should be 
> plenty of sample code to have that easily running.
> 
> Sterling
> 
> On 11 Jul 2016, at 9:25, Christopher Collins wrote:
> 
>> On Mon, Jul 11, 2016 at 08:51:10AM -0700, will sanfilippo wrote:
>>> I have mixed feelings about comments. In my view, it is OK to not
>>> comment the code heavily if there is a document that explains the
>>> code. Either is sufficient in my opinion. Of course, keeping to
>>> Doxygen style comments for public API is a good idea. Do we run
>>> doxygen automatically and can we see what the output looks like for
>>> mynewt? I generally use doxygen style comments for all of my functions
>>> but I have to admit I am not always consistent.
>> 
>> I think comments fall into two broad categories:
>> 
>> 1. A specification of a function's or data structure's contract.
>> 2. Explanatory comments that are intended to make code easier to follow.
>> 
>> The first group of comments generally appear above a function or data
>> type definition and use the doxygen format.  The second group is usually
>> in-line with a function body.
>> 
>> In my opinion, comments of the first variety are essential.  Someone
>> should not have to read code just to understand what inputs are allowed
>> and whether the function consumes a supplied resource on error, for
>> example.  In my opinion, there is no justification for not requiring
>> these type of comments.
>> 
>> I don't have a strong opinion about the second kind of comments.  On the
>> plus side, when they are accurate and up to date, they can be quite
>> helpful for a person reading unfamiliar code.  Unfortunately, these
>> comments are often inaccurate and out of date, they can a slight
>> hindrance in reading for someone already familiar with the code, and
>> worst of all, they make it harder to change code when it needs to be
>> changed.  So I am happy with letting developers use this type of comment
>> at their own discretion.
>> 
>>> The other thing about comments and documentation: it is not easy to
>>> keep them in sync with the actual code. People change things and then
>>> the comments/documents get out of sync. While this is not a reason to
>>> not document/comment, it can sometimes be worse than having no
>>> comments/documentation.
>> 
>> Agreed.  The online documentation will always be out of date unless
>> there is an automated process to convert comments to documentation.
>> 
>>> The issue is always about enforcement; I think we need to have a
>>> conversation about how (and if we should) we enforce adherence to the
>>> coding standards we create.
>> 
>> Insofar as coding standards are useful at all, they need to be enforced.
>> If someone thinks an exception is warranted, they should provide
>> justification in the pull request or commit message.
>> 
>> Yes, this is just a lot of empty talk from me :).  I don't know the
>> right way to enforce coding standards without scaring off contributors.
>> 
>> Chris

--
David G. Simmons
(919) 534-5099
Web • Blog • Linkedin • Twitter • GitHub
/** Message digitally signed for security and authenticity.
* If you cannot read the PGP.sig attachment, please go to
 * http://www.gnupg.com/ Secure your email!!!
 * Public key available at keyserver.pgp.com
**/
♺ This email uses 100% recycled electrons. Don't blow it by printing!

There are only 2 hard things in computer science: Cache invalidation, naming 
things, and off-by-one errors.


Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Reply via email to