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

[John Mellor]

On Fri, Jul 6, 2012 at 11:49 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> On Jul 6, 2012 3:16 PM, "Per Bothner" <per.bothner at 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.bothner at 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...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120711/ed0c371d/attachment.html>