[parted-devel] [PATCH] next branch

[Jim Meyering]

Joel Granados wrote:
> On Tue, May 26, 2009 at 03:16:35PM +0200, Jim Meyering wrote:
>> Joel Granados wrote:
>> >> Regarding the logs,
>> >> when you add a new function, just saying "New function."
>> >> is enough, because there has to be a comment in the
>> >> code already saying what the function does.
>> >
>> > This is true. But I have to insist on a small description, instead of
>> > just saying "New function" as the diff that accompanies the commit might
>> > not be as readable or as obvious.  With that said, I'm OK with that
>> > change.
>>
>> The name is there already.
>> I hope that's enough for you, in general
>>
>> It's better not to describe it at all in the commit log).
>> Otherwise, you risk someone reading and relying on that abbreviated
>> (or maybe inaccurate) description.  In addition, with any duplication
>> at all, you'd want to keep them in sync, but the git log is not
>> modifiable.
>
> True, the git log is not modifyable but we should not be allowing
> inaccurate descriptions anyway.  If the description is inacurate, the
> patch should be rejected.

Sometimes I write a patch and keep it on an unofficial branch for months.
While it's on that branch it may evolve.  It would be annoying (and easy
to forget) to have to adjust comments-in-log to reflect already adjusted
comments-in-code every time that happens.

> You are also correct when you assume that the description might
> become inaccurate when someone changes the function in the future.
> But I would still argue that a small description is needed when the

Description specific to each added function belongs in the code.
Why duplicate that in the log?  The log is the right place for high level
"why make this change?" discussion.  For example, if this patch is mostly
preparation for a subsequent change, there's no point in writing that
in the code.  However it's essential to say it in the log.  Similarly,
when one commit reverts part of another, the log is the place to write
about it.  Reference the affected SHA1, too.  However, in this case,
it's good also to add comments in the code, so that future developers
have a clue if/when they consider reintroducing the bug.

> function name leaves some obvious unanswered questions.  These, of
> course, are situations that must be evaluated per commit.
>
>>
>> Also, it's best to avoid putting too much per-function
>> description in the log, because that could lull the author
>> into thinking s/he has already written the required
>> documentation, without realizing that it was solely
>> for the log, and not yet made into the mandatory
>> function-describing comment.
>
> With this comment I have an opinion and a question:
>
> Question:  One rights documentation of a function when the function is
> not obvious and when the function is part of the library.  right?
> More specifically, What are the policies for documenting a function?

IMHO, nearly every function requires some minimal comment
describing its functionality, the role of each argument,
and the return value.  That comment goes just before the
function definition.  Of course, if it does something
involved or non-obvious, more in-functions comments are
appropriate.

> Opinion:  I think that there are various resons for not documenting
> a function.  Placing a short "doubt resolving" comment in the commit, is

There are very few functions for which a minimal comment would not help.
Please write them unconditionally, at least for parted.

> not one of the main ones.  I would argue that not knowing the policy
> would be a major reason (from the patch creator and from the patch
> reviewer).  Also, laziness, lack of time also pop into my mind as
> reasons that would come before being lulled by the commit message.  I
> would further argue that with the correct knowledge of when to document
> a function, the reviewer would easily detect when a patch needs to add
> documentation.

If I don't write a comment it's because I forget to or ran out of time.
If I commit a new function with no description, please call me on it.
I find that the discipline of writing a good function-describing comment
often clarifies in my mind what the functionality should be, highlights
an inadequate parameter name, or suggests some other interface change.

Regarding *requiring* comments, when it is not possible to cajole
a contributor into providing -- or even just outlining -- a test case,
I wonder if it's worth asking for comments.  Perhaps better for the
maintainers to write them.

Of course, if contribution guidelines were to state the goal,
it would be easier to request the additional few lines per function.