On Fri, Jul 6, 2012 at 11:49 PM, Ryosuke Niwa <rn...@webkit.org> wrote:
> On Jul 6, 2012 3:16 PM, "Per Bothner" <per.both...@oracle.com> wrote: > > On 07/06/2012 02:02 PM, Ryosuke Niwa wrote: > >> On Fri, Jul 6, 2012 at 12:54 PM, Per Bothner <per.both...@oracle.com> > wrote: > >>> 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. > > To be fair, as I said, I was able to read WebKit code without much trouble > 2-3 months into my internship. I'm not sympathetic to people who can't read > code as well as an undergraduate college student. > > Also, we need require some kind of prior knowledge of things. For example, > InlineBox might be a foreign concept to you but it's plain dead obvious for > anyone who has read CSS2.1 specifications. > > Similarly, there might be some classes that are self-explanatory for > someone who knows ES5 well and confusing for someone who is not. > > Where should we draw a line then? > Even obvious (to some) concepts like InlineBox have subtleties, for example not all inline-level elements have inline boxes. An unambiguous class-level comment could make this clearer, for example: // An inline box represents a rectangle that occurs on a line, corresponding to // all or part of some RenderObject. It must be inline-level and its contents // must participate in its containing inline formatting context. For example a // non-replaced element with a 'display' value of 'inline' generates an inline // box, as does an anonymous inline element (text directly contained inside a // block container element, not inside an inline element). But atomic // inline-level boxes (such as replaced inline-level elements, inline-block // elements, inline-table elements, and ruby elements) are not inline boxes // since they participate in their inline formatting context as a single // opaque box; these are handled by <insert class that deals with these>. // http://www.w3.org/TR/2011/REC-CSS2-20110607/visuren.html#inline-boxes Some may find that too verbose. But at least a link to the relevant part of the spec would be a good start...
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev