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