[webkit-dev] Comments in the code (Was Please include function-level comments in change log entries)

[Ryosuke Niwa]

As Eric pointed out, this is completely off the topic of the thread now. So
moving to a new thread.

On Fri, Jul 6, 2012 at 12:54 PM, Per Bothner <per.bothner at oracle.com> wrote:

> On 07/06/2012 11:41 AM, Ryosuke Niwa wrote:
>
>> Indeed, we try to avoid adding comments as much as possible since
>> comments tend to get out-of-date very quickly, we don't want to be
>> spending all our time updating comments.
>>
>
> Heavens forbid that someone who actually understands the code should have
> to update the comments once in a while.  Better to keep it inscrutable
> so newbies spend all of *their* time trying to figure it all out.


That's good, right? People who are new to the project should be reading the
code to understand what each class/function does instead of reading the
comment and deluding himself as if he or she understands the code.

 Instead, we try to refactor
>> code so that code is self-evident or add assertions to codify the
>> comments.
>>
>
> You're deluding yourself if you think the code (or any code this large and
> complicated) is or can be self-evident.


That's a pretty strong claim. I find that the vast majority of codebase to
be quite self-evident. Now, it seems like you do work on JSC, and I
understand JS JIT and JS interpreters could be very complicated.

If you think the code in JSC or other parts of WebKit, then please file
bugs to refactor such code to make them more self-explanatory. I hope we
can both agree that code such as:
Node* highestAncestor(Node* node)
{
    ASSERT(node);
    Node* parent = node;
    while (node = node->parentNode())
        parent = node;
    return parent;
}
doesn't need any comments. We should strive to make all code as
self-evident as this one.

I find it quite painful to figure out my way through the WebKit code-base,
> and I'm hardly inexperienced.
>

I started working on WebKit as a college intern. By the second or the third
months of my internship, I had very little trouble reading the case base.
Of course, things may have been different if I had worked on JSC, line
layout, rendering code, etc... because they tend to be much more
complicated but, nevertheless, your anecdotal evidence doesn't give us a
reason to add more comments.

The biggest annoyance I found is lack of class-level comments.


The lack of class-level comments was also brought up as a significant
contribution barrier the last time we discussed this as well. So perhaps we
need to re-consider adding some class-level comments for some classes.

For example what is an Interpreter?


Interpreter is http://en.wikipedia.org/wiki/Interpreter_(computing), right?
I would be surprised if it weren't.

How many instances are there in the system?
> (I.e. is it a singleton class?  Is there one per window? One per thread?)
>

Okay, I had never heard of this class so I opened up
JavaScriptCore.xcodeproj and searched for "new Interpreter". JSGlobalData
has it as a member so clearly it's not a singleton class. And there's a
nice comment at the beginning of the class declaration of JSGlobalData that
says WebCore has a one-to-one mapping of threads to JSGlobalData.

What is the relationship to JSGlobalData, JSGlobalObject, RootObject.
>

I can't tell the distinction between JSGlobalData and JSGlobalObject either
so maybe we should rename these classes. But then I'm not a JSC expert and
there might be a good reason for their seemingly vague names.

There are a lot of these classes, and it takes quite a bit of staring at
> the code to figure it out. Worse, it's hard to remember it all, so if I
> come back to the codebase after working on something else I have to
> figure out all out again: I might remember some aspects (like a class
> starting with JS is probably some kind of JavaScript object), but not
> a lot of other relationships and properties of the classes.
>
> Those of you who work on WebKit all the time might be comfortable
> with the lack of comments, but I think it's a misguided and unfriendly
> policy.
>

I agree that WebKit codebase may not be the most friendly project to
someone who is new to the project.

On the other hand, we don't want to have valuable contributors spend all
their time updating & adding comments. And the lack of comments doesn't
seem to be a show stopper for people who frequently contribute and make
significant contributions to the project.

- Ryosuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120706/c27a7c0c/attachment.html>