On 07/06/2012 02:02 PM, Ryosuke Niwa wrote:
As Eric pointed out, this is completely off the topic of the thread now.
So moving to a new thread.
[Alas we now have 3 new treads ...]
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.
Yes and no. Trying to infer structure and intention from chasing
calls and cross-references in the code is tedious and error-prone.
I may think I understanding the code, but I may miss some critical
dependency and prerequisite - something that would be obvious to
someone who understand the code.
Furthermore, expecting someone new to "grok" the whole picture before
they can get started is rather intense. And it's not just beginners:
I doubt anyone can have a full accurate mental model of every class and
dependency of WebKit, so some shortcuts help every developer. Good class
and method names are very valuable, but so are well-chosen 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.
If you (who I gather has quite a bit of WebKit experience) find something
self-evident, does not mean it is is.
Now, it seems like you do work on JSC, and I
understand JS JIT and JS interpreters could be very complicated.
Yep. I certainly don't claim to understand all or even most of it.
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 agree - at least I can't think of anything useful to add.
For example what is an Interpreter?
...
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.
Right - we can figure a lot of this stuff out by searching through the
code. But each of these searches may take 5 minutes, and there may be
be dozens of these questions one needs to answer to understand how it
all fits together. Then an hour later you lose track of where you started,
so you need to take notes. So you're tempted to submit a patch so you
didn't have to do this next time you want to figure it out - assuming
you feel such patches are welcome (and you've worked out an acceptable
procedure with your employer for submitting patches ...).
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.
We all have limited resources, and must prioritize. But I think keeping
useful comments up-to-date shouldn't take a lot of time. If nothing else
it's a useful sanity check: If something changes enough that you need to
change a comment, that suggests it's a good thing to take the time to
think through the implications well enough to write them down.
--
--Per Bothner
per.both...@oracle.com p...@bothner.com http://per.bothner.com/
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
