[webkit-dev] coding style and comments

[Dirk Pranke]

Two other ways to potentially answer these questions:

1) What are files that you all think *are* well commented? (pointing
at files that aren't probably isn't that useful, and is certainly
meaner ;).

2) One file over in that same directory, is a "filesystem.py" file
that I wrote (most of). I think it's pretty well commented, by python
standards, but I'm curious what others think. I give you permission to
publicly bash me ahead of time ;)

I will also note, as a general thought, that comments should not be
used as documentation where tests can be used instead. Similarly,
people often under-document tests and make them very hard to maintain.

Lastly, I volunteer to take whatever wisdom is offered up on this
thread and aggregate it onto the Wiki ...

-- Dirk

On Thu, Jan 27, 2011 at 4:38 PM, Dirk Pranke <dpranke at chromium.org> wrote:
> I agree with all of the sentiments below, except for possibly one or two cases.
>
> Namely, sometimes you write code where the "what" is not obvious. In
> that situation, it may not be possible to change the code to make it
> more obvious, because you are bound by contract. For example, suppose
> you are implementing an interface with a method called sort(). The
> fact that you choose a quicksort or a mergesort is a useful one-line
> comment, and in my opinion is probably better than just wrapping a
> method called quicksort() and hoping it gets inlined away.
>
> A slight variant of this is that you may be learning or
> reverse-engineering code where the "what" is not clear (perhaps
> because it was poorly rewritten). In the longer-term, ideally you'd
> clean up the code, but in the short term it might be useful to capture
> the "what" in comments until you can safely refactor things and delete
> the comments.
>
> For example, evmar at chromium.org commented on IRC that he often checks
> out files from the tree, adds local comments to the file, to help keep
> track of what the code does, but then deletes them prior to checking
> his patches back in. He's a pretty good guy, so I am guessing that his
> comments would be useful to others as well (a net improvement to the
> code and an incremental step towards the refactoring described above),
> and it's a shame that that effort is wasted.
>
> Two other specific examples, from the Python code I alluded to in my
> first message:
>
> In Tools/Scripts/webkitpy/common/system, there are the "fileset.py",
> "directoryfileset.py", and "zipfileset.py" files. They are otherwise
> fine but have no comments in them, probably because the contributor
> felt that comments were not "webkit style". However, it took me a
> non-trivial amount of time -- and a brief conversation with Mihai, who
> reviewed the code -- to understand the structure and rationale of the
> code (what is a fileset for, etc.). A couple of file-level or
> class-level comments would've eliminated that concern and may save
> others the same time in the future.
>
> Secondly, in zipfileset.py, there is an open() method (line 49). What
> that method does is entirely non-obvious: It extracts a named member
> of a zip file and saves it into a file named according to the
> "filename" parameter. I think this routine is an example of something
> that would benefit from both either a "what" comment (or a renaming of
> the method, but it's not obvious to me what it should be renamed to,
> perhaps extract_as or extract_and_save_as) and a "why" comment
> (because we need to take the "-actual" files from the layout test
> archives and rename them to the "-expected" versions.
>
> -- Dirk
>
> [ Note that this is not meant to be a mark against that code, which is
> generally pretty good IMO. I would have similar feedback on virtually
> any C++ file I could open up, I suspect. This just happened to be the
> file that prompted the train of thought. ]
>
> On Thu, Jan 27, 2011 at 3:46 PM, Darin Adler <darin at apple.com> wrote:
>> On the WebKit project we like “why” comments, but not “what” comments.
>> Comments that say what the next block of code does usually are not a good
>> idea. Instead by using appropriate names and granularity we want the code
>> itself to say what the comment would have.
>> We also frown on “textbook” style comments. Long block comments that read
>> like a manifesto about what a code or class will do aren’t typical in
>> WebKit.
>> It’s critical that comments contain information that the code does not,
>> rather than repeating information that is already present in the code. And
>> that comments are brief enough and easy enough to read that they are likely
>> to be noticed and updated as the code changes.
>> I don’t think this has much to do with specific programming languages.
>> A good discussion of comments could revolve around some specific code
>> sequences and a discussion of whether a particular comment is suitable. I’m
>> not sure we can get very far in the abstract.
>>     -- Darin
>>
>