Re: [webkit-dev] PSA: rebaseline tooling

[Ojan Vafai]

If someone feels like doing extra hacking to improve this, I'm happy to
walk you through the code. It'd be awesome if you could expand out that
section to show which revision has been run on each bot.
https://bugs.webkit.org/show_bug.cgi?id=91163


On Thu, Jul 12, 2012 at 4:18 PM, Peter Kasting  wrote:

> On Thu, Jul 12, 2012 at 4:16 PM, Dirk Pranke  wrote:
>
>> At the top of the garden-o-matic page there is a line like "Latest
>> revision processed by every bot: 122499 (trunk is at 122524)". I think
>> that does what you want?
>
>
> Ah, I hadn't noticed that.  That sounds promising!
>
> PK
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] PSA: rebaseline tooling

[Peter Kasting]

On Thu, Jul 12, 2012 at 4:16 PM, Dirk Pranke  wrote:

> At the top of the garden-o-matic page there is a line like "Latest
> revision processed by every bot: 122499 (trunk is at 122524)". I think
> that does what you want?


Ah, I hadn't noticed that.  That sounds promising!

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] PSA: rebaseline tooling

[Dirk Pranke]

At the top of the garden-o-matic page there is a line like "Latest
revision processed by every bot: 122499 (trunk is at 122524)". I think
that does what you want?

On Thu, Jul 12, 2012 at 3:56 PM, Peter Kasting  wrote:
> On Thu, Jul 12, 2012 at 3:17 PM, Ojan Vafai  wrote:
>>
>> https://trac.webkit.org/wiki/Rebaseline
>>
>> I've recently consolidated the various rebaseline commands and made the
>> tool work for non-Chromium ports.
>>
>> I especially like "webkit-patch rebaseline
>> path/to/test/i/just/broke.html", which lets you easily rebaseline the test
>> for all ports once the bots have cycled.
>
>
> Thanks Ojan!
>
> On a random note since you mentioned bots cycling.  Is there any way for
> this tool/garden-o-matic to detect whether the bots it's pulling results
> from have cycled since a particular change?  I've been bitten (and I know
> others have too) by trying to pull new baselines too early and then having
> to do it again, and having the tool tell me, say, the most recent revision
> that it has complete results for would be awesome.
>
> PK
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] PSA: rebaseline tooling

[Peter Kasting]

On Thu, Jul 12, 2012 at 3:17 PM, Ojan Vafai  wrote:

> https://trac.webkit.org/wiki/Rebaseline
>
> I've recently consolidated the various rebaseline commands and made the
> tool work for non-Chromium ports.
>
> I especially like "webkit-patch rebaseline
> path/to/test/i/just/broke.html", which lets you easily rebaseline the test
> for all ports once the bots have cycled.
>

Thanks Ojan!

On a random note since you mentioned bots cycling.  Is there any way for
this tool/garden-o-matic to detect whether the bots it's pulling results
from have cycled since a particular change?  I've been bitten (and I know
others have too) by trying to pull new baselines too early and then having
to do it again, and having the tool tell me, say, the most recent revision
that it has complete results for would be awesome.

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] PSA: rebaseline tooling

[Ojan Vafai]

https://trac.webkit.org/wiki/Rebaseline

I've recently consolidated the various rebaseline commands and made the
tool work for non-Chromium ports.

I especially like "webkit-patch rebaseline path/to/test/i/just/broke.html",
which lets you easily rebaseline the test for all ports once the bots have
cycled.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Maciej Stachowiak]

On Jul 12, 2012, at 1:47 PM, Stephen Chenney  wrote:

> 
> 
> On Thu, Jul 12, 2012 at 3:44 PM, Dirk Pranke  wrote:
> On Thu, Jul 12, 2012 at 10:53 AM, Ryosuke Niwa  wrote:
> > On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney 
> > wrote:
> >>
> >> As several people have shown, it is quite easy to come up with a formula
> >> that shows the cost of maintaining comments is much lower than the cost of
> >> living without.
> >
> >
> > I object to that conclusion. I've never seen any scientifically sound data
> > posted on this thread either for or against having comments. Furthermore,
> > just because we can come up with a formula doesn't mean that the formula
> > models the nature of the world well.
> 
> This is certainly true. I doubt you will see such a study, because
> it's very culturally-specific (in the sense that every group working
> on a shared code base is a culture).
> 
> I should have been clearer. In this email thread there have been guesstimates 
> of the form:
> 
> Cost per year with poor commenting: t_understandWithoutComment * 
> n_engineersNeedingToUnderstand
> 
> Cost per year with good comments:  t_maintainComments * n_patches + 
> t_understandWithComment * n_engineersNeedingToUnderstand
> 
> All costs are "per-code unit" and will likely vary depending on the code. We 
> are better off without comments if:
> 
> t_understandWithoutComment < t_maintainComments * n_patches / n_engineers + 
> t_understandWithComment
> 
> We can argue about the appropriate values for t_* and n_*. The primary 
> observation is that the benefit of comments rises as more engineers need to 
> understand the code and patch levels (behavior changes) stay reasonably 
> constant. More behavioral changes argue for fewer comments.

Surely we would expect the project's n_patches to scale approximately linearly 
with n_engineers. Or if not, I'm not super concerned about helping the 
engineers who are not contributing patches.

You've also failed to account for the cost of misleading comments, and the cost 
of comments that add no information value (like m_refCount++; // increase 
reference count by 1). These can  significantly hurt understandability. You may 
think my example of a low-information comment is a parody, but I've noticed 
that there is a high correlation between people who want to follow a policy of 
"lots of comments" and tendency to add comments almost exactly like that. If 
anyone doubts me, I can privately point you to some examples of comments in 
WebKit code that literally restate what the adjecent line of actual code does. 
I hate reading code like that, because it turns 1-page functions into 3-page 
functions. 

Thus, I'm much more interested in comments that pass the filter of people who 
prefer fewer comments and thus would spend their limited comment budget on ones 
that truly have value, than comments from people who believe in adding lots of 
comments. My Bayesian inference is that comments from the latter group have 
much lower average value per comment. When adding a comment, you should really 
think about whether the value outweighs the cost, just as when adding a line of 
actual functional code.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Thu, Jul 12, 2012 at 2:03 PM, Ryosuke Niwa  wrote:

> On Thu, Jul 12, 2012 at 1:47 PM, Stephen Chenney wrote:
>
>> On Thu, Jul 12, 2012 at 3:44 PM, Dirk Pranke wrote:
>>
>>> On Thu, Jul 12, 2012 at 10:53 AM, Ryosuke Niwa  wrote:
>>> > On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney <
>>> schen...@chromium.org>
>>> > wrote:
>>> >>
>>> >> As several people have shown, it is quite easy to come up with a
>>> formula
>>> >> that shows the cost of maintaining comments is much lower than the
>>> cost of
>>> >> living without.
>>> >
>>> >
>>> > I object to that conclusion. I've never seen any scientifically sound
>>> data
>>> > posted on this thread either for or against having comments.
>>> Furthermore,
>>> > just because we can come up with a formula doesn't mean that the
>>> formula
>>> > models the nature of the world well.
>>>
>>> This is certainly true. I doubt you will see such a study, because
>>> it's very culturally-specific (in the sense that every group working
>>> on a shared code base is a culture).
>>>
>>
>> I should have been clearer. In this email thread there have been
>> guesstimates of the form:
>>
>
> Thanks for the clarification. That makes a lot more sense.
>
> Cost per year with poor commenting: t_understandWithoutComment *
>> n_engineersNeedingToUnderstand
>>
>> Cost per year with good comments:  t_maintainComments * n_patches
>> + t_understandWithComment * n_engineersNeedingToUnderstand
>>
>
> "poor" and "good" are not really useful distinction because we can all
> agree that good comments are good. Also, we can't have all poor comments or
> all good comments.
>
> Thus, I postulate that the following formula will more likely depict the
> world accurately:
> Cost(engineer, numberOfComments) = timeToReadCodeWithoutComments(engineer,
> numberOfComments) + timeToReadCodeWithComments(engineer, numberOfComments)
> + timeToMaintainComments(engineer, numberOfComments) +
> timeWastedOnWrongComments(engineer, numberOfComments)
>
> Cost(engineers, numberOfComments) = sum of cost(engineer,
> numberOfComments) over engineers.
>
> Even in this model, there is a gross simplification that only the number
> of comments, not quality or the length of each comment, matters. We need
> some experiments to see whether such a simplification can be acceptable or
> not. Now, let us further assume that all engineers are equal, or that we
> can someone determine the "average" engineer among us (pending another
> experiment here). So the above formula reduces to:
>
> Cost'(numberOfComments) = numberOfEngineers ×
> (timeToReadCodeWithoutComments(engineer, numberOfComments) +
> timeToReadCodeWithComments(engineer, numberOfComments)
> + timeToMaintainComments(engineer, numberOfComments) +
> timeWastedOnWrongComments(engineer, numberOfComments))
>

where engineer here is the "average" engineer.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Thu, Jul 12, 2012 at 1:47 PM, Stephen Chenney wrote:

> On Thu, Jul 12, 2012 at 3:44 PM, Dirk Pranke  wrote:
>
>> On Thu, Jul 12, 2012 at 10:53 AM, Ryosuke Niwa  wrote:
>> > On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney <
>> schen...@chromium.org>
>> > wrote:
>> >>
>> >> As several people have shown, it is quite easy to come up with a
>> formula
>> >> that shows the cost of maintaining comments is much lower than the
>> cost of
>> >> living without.
>> >
>> >
>> > I object to that conclusion. I've never seen any scientifically sound
>> data
>> > posted on this thread either for or against having comments.
>> Furthermore,
>> > just because we can come up with a formula doesn't mean that the formula
>> > models the nature of the world well.
>>
>> This is certainly true. I doubt you will see such a study, because
>> it's very culturally-specific (in the sense that every group working
>> on a shared code base is a culture).
>>
>
> I should have been clearer. In this email thread there have been
> guesstimates of the form:
>

Thanks for the clarification. That makes a lot more sense.

Cost per year with poor commenting: t_understandWithoutComment *
> n_engineersNeedingToUnderstand
>
> Cost per year with good comments:  t_maintainComments * n_patches
> + t_understandWithComment * n_engineersNeedingToUnderstand
>

"poor" and "good" are not really useful distinction because we can all
agree that good comments are good. Also, we can't have all poor comments or
all good comments.

Thus, I postulate that the following formula will more likely depict the
world accurately:
Cost(engineer, numberOfComments) = timeToReadCodeWithoutComments(engineer,
numberOfComments) + timeToReadCodeWithComments(engineer, numberOfComments)
+ timeToMaintainComments(engineer, numberOfComments) +
timeWastedOnWrongComments(engineer, numberOfComments)

Cost(engineers, numberOfComments) = sum of cost(engineer, numberOfComments)
over engineers.

Even in this model, there is a gross simplification that only the number of
comments, not quality or the length of each comment, matters. We need some
experiments to see whether such a simplification can be acceptable or not.
Now, let us further assume that all engineers are equal, or that we can
someone determine the "average" engineer among us (pending another
experiment here). So the above formula reduces to:

Cost'(numberOfComments) = numberOfEngineers ×
(timeToReadCodeWithoutComments(engineer, numberOfComments) +
timeToReadCodeWithComments(engineer, numberOfComments)
+ timeToMaintainComments(engineer, numberOfComments) +
timeWastedOnWrongComments(engineer, numberOfComments))

And the optimization problem we're solving is the minimization
of Cost'(numberOfComments) over numberOfComments.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Stephen Chenney]

On Thu, Jul 12, 2012 at 3:44 PM, Dirk Pranke  wrote:

> On Thu, Jul 12, 2012 at 10:53 AM, Ryosuke Niwa  wrote:
> > On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney  >
> > wrote:
> >>
> >> As several people have shown, it is quite easy to come up with a formula
> >> that shows the cost of maintaining comments is much lower than the cost
> of
> >> living without.
> >
> >
> > I object to that conclusion. I've never seen any scientifically sound
> data
> > posted on this thread either for or against having comments. Furthermore,
> > just because we can come up with a formula doesn't mean that the formula
> > models the nature of the world well.
>
> This is certainly true. I doubt you will see such a study, because
> it's very culturally-specific (in the sense that every group working
> on a shared code base is a culture).
>

I should have been clearer. In this email thread there have been
guesstimates of the form:

Cost per year with poor commenting: t_understandWithoutComment *
n_engineersNeedingToUnderstand

Cost per year with good comments:  t_maintainComments * n_patches
+ t_understandWithComment * n_engineersNeedingToUnderstand

All costs are "per-code unit" and will likely vary depending on the code.
We are better off without comments if:

t_understandWithoutComment < t_maintainComments * n_patches / n_engineers
+ t_understandWithComment

We can argue about the appropriate values for t_* and n_*. The primary
observation is that the benefit of comments rises as more engineers need to
understand the code and patch levels (behavior changes) stay reasonably
constant. More behavioral changes argue for fewer comments.

We can all have fun fiddling the numbers to make our cases. Go for it in
the privacy of your own home.

For anything that looks like "core" code where lots of engineers need to
understand it, I think commenting wins most of the time. My current
favorite is "inDocument()". For peripheral code with frequent changes in
behavior, it's not worth it. There's a big grey area in between (like
Ryosuke's editing code) where the best approach is not obvious.

As an aside, in WebKit, I think the culture is actually actually
> anti-comment enough that it's trained some people to not read comments
> at all, and so the value of comments is even lower. I have often had
> people ask questions about code in reviews that were answered by
> comments right above the lines in question :).
>
> -- Dirk
>

Absolutely. At heart this is a cultural issue and we could let it all play
out by pushing our individual opinions in patches and reviews. Over time
we'll see what happens.

Cheers,
Stephen.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Eric Seidel]

I for one, very much appreciate your cleanup/organizational efforts!

On Thu, Jul 12, 2012 at 12:37 PM, Alexis Menard
 wrote:
> So far in the css/ directory we tried to renamed slowly classes so that :
>
> CSS* prefixed classes are the implementation of CSSOM
> "whatevername" is for internal classes. For example we renamed
> CSSStyleApplyProperty class to StyleBuilder because it's internal.
>
> Hope that helps.
>
> On Thu, Jul 12, 2012 at 2:52 PM, Simon Fraser  wrote:
>> I'd prefer we keep "Region" for the low-level graphics primitive Region
>> (just like Path), and use something prefixed for the higher-level layout
>> concept.
>>
>> Simon
>>
>> On Jul 12, 2012, at 10:26 AM, Dana Jansens wrote:
>>
>> On Thu, Jul 12, 2012 at 1:25 PM, Ryosuke Niwa  wrote:
>>>
>>> I'd vote for CSSRegion or CSSOMRegion for the class you're adding but I'll
>>> also suggest we rename the existing Region class now that the term "region"
>>> has a specific semantic in CSS. Maybe LayoutRegion or ScreenRegion?
>>
>> IntRegion? It seems closer to an IntRect than a LayoutRect.
>>>
>>> - Ryosuke
>>>
>>> On Jul 12, 2012 10:13 AM, "Eric Seidel"  wrote:

 I would go with CSSRegion, and stick it in the css/ folder.  Much of
 the CSS folder is our implementation of the CSS Object Model (CSSOM).
 At some point it might make sense to pull all the classes which
 implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
 that's a later discussion.

 -eric

 On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
 > From my knowledge the "CSS" prefix is reserved for the CSS engine
 > classes in
 > WebKit. Prefixing the Region class with "CSS" could prove confusing.
 >
 > Regards,
 > Andrei.
 >
 > From: Alan Stearns 
 > Date: Thursday, July 12, 2012 7:39 PM
 > To: Adam Barth , Andrei Bucur 
 >
 > Cc: "webkit-dev@lists.webkit.org" 
 > Subject: Re: [webkit-dev] Web APIs and class name collisions
 >
 > The spec itself consistently and deliberately calls them "CSS Regions,"
 > so a
 > CSS prefix could be appropriate.
 >
 > Thanks,
 >
 > Alan
 >
 >
 > From: Adam Barth 
 > To: Andrei Bucur 
 > Cc: "webkit-dev@lists.webkit.org" 
 > Subject: Re: [webkit-dev] Web APIs and class name collisions
 >
 > One common thing we do is prefix "DOM" to DOM-level concepts.  For
 > example,
 > DOMWindow and DOMFileSystem.  I'm not sure if we have an established
 > convention for CSS-level concepts.
 >
 > Adam
 >
 >
 > On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
 >>
 >> Hello Webkittens!
 >>
 >> While implementing the Region interface (
 >> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've
 >> noticed
 >> that the name "Region" is already taken by a class in
 >> platform/graphics. I'd
 >> like to know what's the best approach in these kind of situations:
 >>
 >> Rename the existing WebCore class to something else and use the name
 >> "Region" for the Web API so there's parity between the implementation
 >> and
 >> the spec
 >> Somehow prefix the Web API implementation class name?
 >>
 >> As the Web APIs expand I suppose this situation may occur again in the
 >> future and I suppose there should be a rule describing what's the best
 >> approach to take.
 >>
 >> Thanks!
 >> Andrei.
 >>
 >> ___
 >> webkit-dev mailing list
 >> webkit-dev@lists.webkit.org
 >> http://lists.webkit.org/mailman/listinfo/webkit-dev
 >>
 >
 >
 > ___
 > webkit-dev mailing list
 > webkit-dev@lists.webkit.org
 > http://lists.webkit.org/mailman/listinfo/webkit-dev
 >
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev
>>>
>>>
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>>
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>>
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
>
>
> --
> Alexis Menard (darktears)
> Software Engineer
> openBossa @ INdT - Instituto Nokia de Tecnologia
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webk

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

[Dirk Pranke]

On Thu, Jul 12, 2012 at 10:53 AM, Ryosuke Niwa  wrote:
> On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney 
> wrote:
>>
>> As several people have shown, it is quite easy to come up with a formula
>> that shows the cost of maintaining comments is much lower than the cost of
>> living without.
>
>
> I object to that conclusion. I've never seen any scientifically sound data
> posted on this thread either for or against having comments. Furthermore,
> just because we can come up with a formula doesn't mean that the formula
> models the nature of the world well.

This is certainly true. I doubt you will see such a study, because
it's very culturally-specific (in the sense that every group working
on a shared code base is a culture).

All code reading is training; you have to learn the styles and idioms
of the codebase.

As an aside, in WebKit, I think the culture is actually actually
anti-comment enough that it's trained some people to not read comments
at all, and so the value of comments is even lower. I have often had
people ask questions about code in reviews that were answered by
comments right above the lines in question :).

-- Dirk
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Alexis Menard]

So far in the css/ directory we tried to renamed slowly classes so that :

CSS* prefixed classes are the implementation of CSSOM
"whatevername" is for internal classes. For example we renamed
CSSStyleApplyProperty class to StyleBuilder because it's internal.

Hope that helps.

On Thu, Jul 12, 2012 at 2:52 PM, Simon Fraser  wrote:
> I'd prefer we keep "Region" for the low-level graphics primitive Region
> (just like Path), and use something prefixed for the higher-level layout
> concept.
>
> Simon
>
> On Jul 12, 2012, at 10:26 AM, Dana Jansens wrote:
>
> On Thu, Jul 12, 2012 at 1:25 PM, Ryosuke Niwa  wrote:
>>
>> I'd vote for CSSRegion or CSSOMRegion for the class you're adding but I'll
>> also suggest we rename the existing Region class now that the term "region"
>> has a specific semantic in CSS. Maybe LayoutRegion or ScreenRegion?
>
> IntRegion? It seems closer to an IntRect than a LayoutRect.
>>
>> - Ryosuke
>>
>> On Jul 12, 2012 10:13 AM, "Eric Seidel"  wrote:
>>>
>>> I would go with CSSRegion, and stick it in the css/ folder.  Much of
>>> the CSS folder is our implementation of the CSS Object Model (CSSOM).
>>> At some point it might make sense to pull all the classes which
>>> implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
>>> that's a later discussion.
>>>
>>> -eric
>>>
>>> On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
>>> > From my knowledge the "CSS" prefix is reserved for the CSS engine
>>> > classes in
>>> > WebKit. Prefixing the Region class with "CSS" could prove confusing.
>>> >
>>> > Regards,
>>> > Andrei.
>>> >
>>> > From: Alan Stearns 
>>> > Date: Thursday, July 12, 2012 7:39 PM
>>> > To: Adam Barth , Andrei Bucur 
>>> >
>>> > Cc: "webkit-dev@lists.webkit.org" 
>>> > Subject: Re: [webkit-dev] Web APIs and class name collisions
>>> >
>>> > The spec itself consistently and deliberately calls them "CSS Regions,"
>>> > so a
>>> > CSS prefix could be appropriate.
>>> >
>>> > Thanks,
>>> >
>>> > Alan
>>> >
>>> >
>>> > From: Adam Barth 
>>> > To: Andrei Bucur 
>>> > Cc: "webkit-dev@lists.webkit.org" 
>>> > Subject: Re: [webkit-dev] Web APIs and class name collisions
>>> >
>>> > One common thing we do is prefix "DOM" to DOM-level concepts.  For
>>> > example,
>>> > DOMWindow and DOMFileSystem.  I'm not sure if we have an established
>>> > convention for CSS-level concepts.
>>> >
>>> > Adam
>>> >
>>> >
>>> > On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
>>> >>
>>> >> Hello Webkittens!
>>> >>
>>> >> While implementing the Region interface (
>>> >> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've
>>> >> noticed
>>> >> that the name "Region" is already taken by a class in
>>> >> platform/graphics. I'd
>>> >> like to know what's the best approach in these kind of situations:
>>> >>
>>> >> Rename the existing WebCore class to something else and use the name
>>> >> "Region" for the Web API so there's parity between the implementation
>>> >> and
>>> >> the spec
>>> >> Somehow prefix the Web API implementation class name?
>>> >>
>>> >> As the Web APIs expand I suppose this situation may occur again in the
>>> >> future and I suppose there should be a rule describing what's the best
>>> >> approach to take.
>>> >>
>>> >> Thanks!
>>> >> Andrei.
>>> >>
>>> >> ___
>>> >> webkit-dev mailing list
>>> >> webkit-dev@lists.webkit.org
>>> >> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>> >>
>>> >
>>> >
>>> > ___
>>> > webkit-dev mailing list
>>> > webkit-dev@lists.webkit.org
>>> > http://lists.webkit.org/mailman/listinfo/webkit-dev
>>> >
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>



-- 
Alexis Menard (darktears)
Software Engineer
openBossa @ INdT - Instituto Nokia de Tecnologia
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

Mark Rowe wrote:

> It'd be complicated to do this more widely since the AvailabilityMacros.h
> version of the macros are defined even when building for iOS. We'd need to
> review all of the uses to ensure that they were handled correctly. Given
> that I think it'd be better to just use this as a localized workaround
> until we drop support for building for Leopard.
>

The one place where we found it to be significant so far is in a
Chromium-specific file that’s not relevant to iOS, and we’re just going to
use MAC_OS_X_VERSION_MAX_ALLOWED in that one instance for the time being.
If it happens anywhere else with this SDK, it’ll likely also be
Chromium-specific, and we’ll just deal with the ugliness of having to use a
different macro for this purpose until we get rid of the 10.5 SDK
altogether. Shouldn’t be long.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Rowe]

On 2012-07-12, at 11:29, Mark Mentovai  wrote:

> We just discovered (via a rollout on bug 91103) that there’s a bug with 
> __MAC_OS_X_VERSION_MAX_ALLOWED in the version of the 10.5 SDK as present in 
> Xcode 3.2.6. This may be the last version of the 10.5 SDK ever released (I 
> haven’t checked all of the early Xcode 4 releases). Chromium uses this SDK 
> for its builds, although will likely switch to the 10.6 SDK in the near 
> future.
> 
> In short, __MAC_OS_X_VERSION_MAX_ALLOWED, which is supposed to track the SDK 
> version, is set to 1060 instead of the expected 1050 in this version of the 
> 10.5 SDK.

This is unfortunate. My understanding is that Chromium will soon be dropping 
Leopard support so this problem should go away in the near future.

> At least for the purposes of testing whether the 10.5 SDK is in use, we’ll 
> need to use ’s MAC_OS_X_VERSION_MAX_ALLOWED instead of 
> ’s __MAC_OS_X_VERSION_MAX_ALLOWED. I guess it’s a question of 
> style whether the rest of WebKit should follow.

It'd be complicated to do this more widely since the AvailabilityMacros.h 
version of the macros are defined even when building for iOS. We'd need to 
review all of the uses to ensure that they were handled correctly. Given that I 
think it'd be better to just use this as a localized workaround until we drop 
support for building for Leopard.

An alternative workaround would be to explicitly define 
__MAC_OS_X_VERSION_MAX_ALLOWED == 1050 in your compiler invocation when 
building against the 10.5 SDK.

- Mark

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removing BUILDING_ON / TARGETING macros in favor of system availability macros

[Mark Mentovai]

We just discovered (via a rollout on bug 91103) that there’s a bug
with __MAC_OS_X_VERSION_MAX_ALLOWED in the version of the 10.5 SDK as
present in Xcode 3.2.6. This may be the last version of the 10.5 SDK ever
released (I haven’t checked all of the early Xcode 4 releases). Chromium
uses this SDK for its builds, although will likely switch to the 10.6 SDK
in the near future.

In short, __MAC_OS_X_VERSION_MAX_ALLOWED, which is supposed to track the
SDK version, is set to 1060 instead of the expected 1050 in this version of
the 10.5 SDK.

--
// clang t.c -isysroot /Xcode3.2/SDKs/MacOSX10.5.sdk
-mmacosx-version-min=10.4 -o t
#include 
#include 
#include 

int main(int argc, char* argv[]) {
#define PRINT_MACRO(m) printf("%s = %d\n", # m, m)
  PRINT_MACRO(MAC_OS_X_VERSION_MIN_REQUIRED);
  PRINT_MACRO(MAC_OS_X_VERSION_MAX_ALLOWED);
  PRINT_MACRO(__MAC_OS_X_VERSION_MIN_REQUIRED);
  PRINT_MACRO(__MAC_OS_X_VERSION_MAX_ALLOWED);
#undef PRINT_MACRO
  return 0;
}
--

produces

MAC_OS_X_VERSION_MIN_REQUIRED = 1040
MAC_OS_X_VERSION_MAX_ALLOWED = 1050
__MAC_OS_X_VERSION_MIN_REQUIRED = 1040
__MAC_OS_X_VERSION_MAX_ALLOWED = 1060

Oops.

I traced this problem to , which in this SDK, says
(slightly abbreviated):

#elif defined(__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__)
#ifndef __MAC_OS_X_VERSION_MAX_ALLOWED
#define __MAC_OS_X_VERSION_MAX_ALLOWED __MAC_10_6
#endif
#endif

At least for the purposes of testing whether the 10.5 SDK is in use, we’ll
need to use ’s MAC_OS_X_VERSION_MAX_ALLOWED instead
of ’s __MAC_OS_X_VERSION_MAX_ALLOWED. I guess it’s a
question of style whether the rest of WebKit should follow.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Ryosuke Niwa]

On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney wrote:

> As several people have shown, it is quite easy to come up with a formula
> that shows the cost of maintaining comments is much lower than the cost of
> living without.
>

I object to that conclusion. I've never seen any scientifically sound data
posted on this thread either for or against having comments. Furthermore,
just because we can come up with a formula doesn't mean that the formula
models the nature of the world well.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Simon Fraser]

I'd prefer we keep "Region" for the low-level graphics primitive Region (just 
like Path), and use something prefixed for the higher-level layout concept.

Simon

On Jul 12, 2012, at 10:26 AM, Dana Jansens wrote:

> On Thu, Jul 12, 2012 at 1:25 PM, Ryosuke Niwa  wrote:
> I'd vote for CSSRegion or CSSOMRegion for the class you're adding but I'll 
> also suggest we rename the existing Region class now that the term "region" 
> has a specific semantic in CSS. Maybe LayoutRegion or ScreenRegion?
> 
> IntRegion? It seems closer to an IntRect than a LayoutRect. 
> - Ryosuke
> 
> On Jul 12, 2012 10:13 AM, "Eric Seidel"  wrote:
> I would go with CSSRegion, and stick it in the css/ folder.  Much of
> the CSS folder is our implementation of the CSS Object Model (CSSOM).
> At some point it might make sense to pull all the classes which
> implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
> that's a later discussion.
> 
> -eric
> 
> On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
> > From my knowledge the "CSS" prefix is reserved for the CSS engine classes in
> > WebKit. Prefixing the Region class with "CSS" could prove confusing.
> >
> > Regards,
> > Andrei.
> >
> > From: Alan Stearns 
> > Date: Thursday, July 12, 2012 7:39 PM
> > To: Adam Barth , Andrei Bucur 
> >
> > Cc: "webkit-dev@lists.webkit.org" 
> > Subject: Re: [webkit-dev] Web APIs and class name collisions
> >
> > The spec itself consistently and deliberately calls them "CSS Regions," so a
> > CSS prefix could be appropriate.
> >
> > Thanks,
> >
> > Alan
> >
> >
> > From: Adam Barth 
> > To: Andrei Bucur 
> > Cc: "webkit-dev@lists.webkit.org" 
> > Subject: Re: [webkit-dev] Web APIs and class name collisions
> >
> > One common thing we do is prefix "DOM" to DOM-level concepts.  For example,
> > DOMWindow and DOMFileSystem.  I'm not sure if we have an established
> > convention for CSS-level concepts.
> >
> > Adam
> >
> >
> > On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
> >>
> >> Hello Webkittens!
> >>
> >> While implementing the Region interface (
> >> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed
> >> that the name "Region" is already taken by a class in platform/graphics. 
> >> I'd
> >> like to know what's the best approach in these kind of situations:
> >>
> >> Rename the existing WebCore class to something else and use the name
> >> "Region" for the Web API so there's parity between the implementation and
> >> the spec
> >> Somehow prefix the Web API implementation class name?
> >>
> >> As the Web APIs expand I suppose this situation may occur again in the
> >> future and I suppose there should be a rule describing what's the best
> >> approach to take.
> >>
> >> Thanks!
> >> Andrei.
> >>
> >> ___
> >> webkit-dev mailing list
> >> webkit-dev@lists.webkit.org
> >> http://lists.webkit.org/mailman/listinfo/webkit-dev
> >>
> >
> >
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo/webkit-dev
> >
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Simon Fraser]

I'd prefer we keep "Region" for the low-level graphics primitive Region (just 
like Path), and use something prefixed for the higher-level layout concept.

Simon

On Jul 12, 2012, at 10:26 AM, Dana Jansens wrote:

> On Thu, Jul 12, 2012 at 1:25 PM, Ryosuke Niwa  wrote:
> I'd vote for CSSRegion or CSSOMRegion for the class you're adding but I'll 
> also suggest we rename the existing Region class now that the term "region" 
> has a specific semantic in CSS. Maybe LayoutRegion or ScreenRegion?
> 
> IntRegion? It seems closer to an IntRect than a LayoutRect. 
> - Ryosuke
> 
> On Jul 12, 2012 10:13 AM, "Eric Seidel"  wrote:
> I would go with CSSRegion, and stick it in the css/ folder.  Much of
> the CSS folder is our implementation of the CSS Object Model (CSSOM).
> At some point it might make sense to pull all the classes which
> implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
> that's a later discussion.
> 
> -eric
> 
> On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
> > From my knowledge the "CSS" prefix is reserved for the CSS engine classes in
> > WebKit. Prefixing the Region class with "CSS" could prove confusing.
> >
> > Regards,
> > Andrei.
> >
> > From: Alan Stearns 
> > Date: Thursday, July 12, 2012 7:39 PM
> > To: Adam Barth , Andrei Bucur 
> >
> > Cc: "webkit-dev@lists.webkit.org" 
> > Subject: Re: [webkit-dev] Web APIs and class name collisions
> >
> > The spec itself consistently and deliberately calls them "CSS Regions," so a
> > CSS prefix could be appropriate.
> >
> > Thanks,
> >
> > Alan
> >
> >
> > From: Adam Barth 
> > To: Andrei Bucur 
> > Cc: "webkit-dev@lists.webkit.org" 
> > Subject: Re: [webkit-dev] Web APIs and class name collisions
> >
> > One common thing we do is prefix "DOM" to DOM-level concepts.  For example,
> > DOMWindow and DOMFileSystem.  I'm not sure if we have an established
> > convention for CSS-level concepts.
> >
> > Adam
> >
> >
> > On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
> >>
> >> Hello Webkittens!
> >>
> >> While implementing the Region interface (
> >> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed
> >> that the name "Region" is already taken by a class in platform/graphics. 
> >> I'd
> >> like to know what's the best approach in these kind of situations:
> >>
> >> Rename the existing WebCore class to something else and use the name
> >> "Region" for the Web API so there's parity between the implementation and
> >> the spec
> >> Somehow prefix the Web API implementation class name?
> >>
> >> As the Web APIs expand I suppose this situation may occur again in the
> >> future and I suppose there should be a rule describing what's the best
> >> approach to take.
> >>
> >> Thanks!
> >> Andrei.
> >>
> >> ___
> >> webkit-dev mailing list
> >> webkit-dev@lists.webkit.org
> >> http://lists.webkit.org/mailman/listinfo/webkit-dev
> >>
> >
> >
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo/webkit-dev
> >
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Stephen Chenney]

On Wed, Jul 11, 2012 at 6:07 PM, Alec Flett  wrote:

> I absolutely do not buy that the "cost" of keeping comments up to date and
> the "cost" of out-of-date comments outweighs the benefits - that has NEVER
> been my experience and if anything the benefits of comments grow as the
> number of people on a project grows. 20 minutes of your time
> documenting/updating comments is worth 10 minutes of 10 (or 100)
> developers' time *not* groveling through code. I also do not buy that all
> code can be written in a self-documenting fashion. That's like saying that
> nobody needs to write books about anything spec'ed by the W3C. Code is not
> documentation no more than a spec is code.
>

The alternatives to comments are themselves subject to the "out-of-date"
problem to an even greater degree. Someone modifying or reviewing code with
comments nearby is far more likely to notice a problem with the comments
than they are to know that some blog post somewhere is now out of date due
to the code change. Even community knowledge is subject to out-of-date
problems, because most people don't look at most patches.

If anything I'm a firm believer that taking the time to write a comment
> forces you to write code correctly the first time. i.e. if I have to write
> in a comment "this is true except in this one esoteric case" then it forces
> me to reexamine that esoteric case and ask myself if that exception is
> really necessary. In fact the way I generally write my code in WebKit is to
> comment heavily so I can keep track of my own intentions, and then just
> before sending off for review, strip all my code of comments, and that's
> truly a waste.
>

This is an excellent and compelling reason to have comments, and
documentation in general.  I cannot count the number of times I have
discovered problems with something at the time I was forced to write about
it. While Changelogs and bug reports are intended to enforce this level of
thinking, in most cases they are not as specific as I would expect comments
to be, particularly for medium to large patches.

Even if the comment is out of date, the time spent verifying that it is
incorrect is no different to the time one should spend verifying that a
change itself is correct.

Finally, arguing that something is bad because it might get out-of-date is
not particularly helpful. Much of the world's information goes out of date,
yet society still chooses to provide it. Everything from transit schedules
to online help pages. The anti-comment argument here is really that it is
not worth the effort of keeping the comments up to date. As several people
have shown, it is quite easy to come up with a formula that shows the cost
of maintaining comments is much lower than the cost of living without.

Cheers,
Stephen.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Dana Jansens]

On Thu, Jul 12, 2012 at 1:25 PM, Ryosuke Niwa  wrote:

> I'd vote for CSSRegion or CSSOMRegion for the class you're adding but I'll
> also suggest we rename the existing Region class now that the term "region"
> has a specific semantic in CSS. Maybe LayoutRegion or ScreenRegion?
>
IntRegion? It seems closer to an IntRect than a LayoutRect.

> - Ryosuke
> On Jul 12, 2012 10:13 AM, "Eric Seidel"  wrote:
>
>> I would go with CSSRegion, and stick it in the css/ folder.  Much of
>> the CSS folder is our implementation of the CSS Object Model (CSSOM).
>> At some point it might make sense to pull all the classes which
>> implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
>> that's a later discussion.
>>
>> -eric
>>
>> On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
>> > From my knowledge the "CSS" prefix is reserved for the CSS engine
>> classes in
>> > WebKit. Prefixing the Region class with "CSS" could prove confusing.
>> >
>> > Regards,
>> > Andrei.
>> >
>> > From: Alan Stearns 
>> > Date: Thursday, July 12, 2012 7:39 PM
>> > To: Adam Barth , Andrei Bucur 
>> >
>> > Cc: "webkit-dev@lists.webkit.org" 
>> > Subject: Re: [webkit-dev] Web APIs and class name collisions
>> >
>> > The spec itself consistently and deliberately calls them "CSS Regions,"
>> so a
>> > CSS prefix could be appropriate.
>> >
>> > Thanks,
>> >
>> > Alan
>> >
>> >
>> > From: Adam Barth 
>> > To: Andrei Bucur 
>> > Cc: "webkit-dev@lists.webkit.org" 
>> > Subject: Re: [webkit-dev] Web APIs and class name collisions
>> >
>> > One common thing we do is prefix "DOM" to DOM-level concepts.  For
>> example,
>> > DOMWindow and DOMFileSystem.  I'm not sure if we have an established
>> > convention for CSS-level concepts.
>> >
>> > Adam
>> >
>> >
>> > On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
>> >>
>> >> Hello Webkittens!
>> >>
>> >> While implementing the Region interface (
>> >> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've
>> noticed
>> >> that the name "Region" is already taken by a class in
>> platform/graphics. I'd
>> >> like to know what's the best approach in these kind of situations:
>> >>
>> >> Rename the existing WebCore class to something else and use the name
>> >> "Region" for the Web API so there's parity between the implementation
>> and
>> >> the spec
>> >> Somehow prefix the Web API implementation class name?
>> >>
>> >> As the Web APIs expand I suppose this situation may occur again in the
>> >> future and I suppose there should be a rule describing what's the best
>> >> approach to take.
>> >>
>> >> Thanks!
>> >> Andrei.
>> >>
>> >> ___
>> >> webkit-dev mailing list
>> >> webkit-dev@lists.webkit.org
>> >> http://lists.webkit.org/mailman/listinfo/webkit-dev
>> >>
>> >
>> >
>> > ___
>> > webkit-dev mailing list
>> > webkit-dev@lists.webkit.org
>> > http://lists.webkit.org/mailman/listinfo/webkit-dev
>> >
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Ryosuke Niwa]

I'd vote for CSSRegion or CSSOMRegion for the class you're adding but I'll
also suggest we rename the existing Region class now that the term "region"
has a specific semantic in CSS. Maybe LayoutRegion or ScreenRegion?

- Ryosuke
On Jul 12, 2012 10:13 AM, "Eric Seidel"  wrote:

> I would go with CSSRegion, and stick it in the css/ folder.  Much of
> the CSS folder is our implementation of the CSS Object Model (CSSOM).
> At some point it might make sense to pull all the classes which
> implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
> that's a later discussion.
>
> -eric
>
> On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
> > From my knowledge the "CSS" prefix is reserved for the CSS engine
> classes in
> > WebKit. Prefixing the Region class with "CSS" could prove confusing.
> >
> > Regards,
> > Andrei.
> >
> > From: Alan Stearns 
> > Date: Thursday, July 12, 2012 7:39 PM
> > To: Adam Barth , Andrei Bucur 
> >
> > Cc: "webkit-dev@lists.webkit.org" 
> > Subject: Re: [webkit-dev] Web APIs and class name collisions
> >
> > The spec itself consistently and deliberately calls them "CSS Regions,"
> so a
> > CSS prefix could be appropriate.
> >
> > Thanks,
> >
> > Alan
> >
> >
> > From: Adam Barth 
> > To: Andrei Bucur 
> > Cc: "webkit-dev@lists.webkit.org" 
> > Subject: Re: [webkit-dev] Web APIs and class name collisions
> >
> > One common thing we do is prefix "DOM" to DOM-level concepts.  For
> example,
> > DOMWindow and DOMFileSystem.  I'm not sure if we have an established
> > convention for CSS-level concepts.
> >
> > Adam
> >
> >
> > On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
> >>
> >> Hello Webkittens!
> >>
> >> While implementing the Region interface (
> >> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've
> noticed
> >> that the name "Region" is already taken by a class in
> platform/graphics. I'd
> >> like to know what's the best approach in these kind of situations:
> >>
> >> Rename the existing WebCore class to something else and use the name
> >> "Region" for the Web API so there's parity between the implementation
> and
> >> the spec
> >> Somehow prefix the Web API implementation class name?
> >>
> >> As the Web APIs expand I suppose this situation may occur again in the
> >> future and I suppose there should be a rule describing what's the best
> >> approach to take.
> >>
> >> Thanks!
> >> Andrei.
> >>
> >> ___
> >> webkit-dev mailing list
> >> webkit-dev@lists.webkit.org
> >> http://lists.webkit.org/mailman/listinfo/webkit-dev
> >>
> >
> >
> > ___
> > webkit-dev mailing list
> > webkit-dev@lists.webkit.org
> > http://lists.webkit.org/mailman/listinfo/webkit-dev
> >
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Eric Seidel]

I would go with CSSRegion, and stick it in the css/ folder.  Much of
the CSS folder is our implementation of the CSS Object Model (CSSOM).
At some point it might make sense to pull all the classes which
implement the CSSOM out of css/ into a new cssom/ similar to dom/, but
that's a later discussion.

-eric

On Thu, Jul 12, 2012 at 10:03 AM, Andrei Bucur  wrote:
> From my knowledge the "CSS" prefix is reserved for the CSS engine classes in
> WebKit. Prefixing the Region class with "CSS" could prove confusing.
>
> Regards,
> Andrei.
>
> From: Alan Stearns 
> Date: Thursday, July 12, 2012 7:39 PM
> To: Adam Barth , Andrei Bucur 
>
> Cc: "webkit-dev@lists.webkit.org" 
> Subject: Re: [webkit-dev] Web APIs and class name collisions
>
> The spec itself consistently and deliberately calls them "CSS Regions," so a
> CSS prefix could be appropriate.
>
> Thanks,
>
> Alan
>
>
> From: Adam Barth 
> To: Andrei Bucur 
> Cc: "webkit-dev@lists.webkit.org" 
> Subject: Re: [webkit-dev] Web APIs and class name collisions
>
> One common thing we do is prefix "DOM" to DOM-level concepts.  For example,
> DOMWindow and DOMFileSystem.  I'm not sure if we have an established
> convention for CSS-level concepts.
>
> Adam
>
>
> On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:
>>
>> Hello Webkittens!
>>
>> While implementing the Region interface (
>> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed
>> that the name "Region" is already taken by a class in platform/graphics. I'd
>> like to know what's the best approach in these kind of situations:
>>
>> Rename the existing WebCore class to something else and use the name
>> "Region" for the Web API so there's parity between the implementation and
>> the spec
>> Somehow prefix the Web API implementation class name?
>>
>> As the Web APIs expand I suppose this situation may occur again in the
>> future and I suppose there should be a rule describing what's the best
>> approach to take.
>>
>> Thanks!
>> Andrei.
>>
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo/webkit-dev
>>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Andrei Bucur]

>From my knowledge the "CSS" prefix is reserved for the CSS engine classes in 
>WebKit. Prefixing the Region class with "CSS" could prove confusing.

Regards,
Andrei.

From: Alan Stearns mailto:stea...@adobe.com>>
Date: Thursday, July 12, 2012 7:39 PM
To: Adam Barth mailto:aba...@webkit.org>>, Andrei Bucur 
mailto:abu...@adobe.com>>
Cc: "webkit-dev@lists.webkit.org" 
mailto:webkit-dev@lists.webkit.org>>
Subject: Re: [webkit-dev] Web APIs and class name collisions

The spec itself consistently and deliberately calls them "CSS Regions," so a 
CSS prefix could be appropriate.

Thanks,

Alan


From: Adam Barth mailto:aba...@webkit.org>>
To: Andrei Bucur mailto:abu...@adobe.com>>
Cc: "webkit-dev@lists.webkit.org" 
mailto:webkit-dev@lists.webkit.org>>
Subject: Re: [webkit-dev] Web APIs and class name collisions

One common thing we do is prefix "DOM" to DOM-level concepts.  For example, 
DOMWindow and DOMFileSystem.  I'm not sure if we have an established convention 
for CSS-level concepts.

Adam


On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur 
mailto:abu...@adobe.com>> wrote:
Hello Webkittens!

While implementing the Region interface ( 
http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed that 
the name "Region" is already taken by a class in platform/graphics. I'd like to 
know what's the best approach in these kind of situations:

 1.  Rename the existing WebCore class to something else and use the name 
"Region" for the Web API so there's parity between the implementation and the 
spec
 2.  Somehow prefix the Web API implementation class name?

As the Web APIs expand I suppose this situation may occur again in the future 
and I suppose there should be a rule describing what's the best approach to 
take.

Thanks!
Andrei.

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[Maciej Stachowiak]

On Jul 12, 2012, at 6:50 AM, John Mellor  wrote:

> 
> 
> To take an arbitrary example, lets say that while iterating through a 
> ListHashSet something causes entries to be deleted. Intuitively it seems this 
> needn't invalidate the iterator, as long as the entry the iterator is 
> currently pointing to isn't removed. But is that actually the case in this 
> particular implementation? A well-documented library like Java's 
> LinkedHashSet will warn you "if the set is modified at any time after the 
> iterator is created, in any way except through the iterator's own remove 
> method, the iterator will throw a ConcurrentModificationException" and that's 
> that. I just tried to find this out in WebKit and had to read though 
> ListHashSetIterator, ListHashSetConstIterator, ListHashSetNode, 
> ListHashSet::remove, ListHashSet::unlinkAndDelete, HashTable::remove, 
> HashTable::internalCheckTableConsistency, 
> HashTable::removeWithoutEntryConsistencyCheck, 
> HashTable::removeAndInvalidateWithoutEntryConsistencyCheck, 
> HashTable::invalidateIterators, HashTable::shrink, HashTable::rehash, 
> ListHashSet::add, HashTable::add, HashTable::makeKnownGoodIterator, 
> HashTableIterator, HashTable::AddResult and ListHashSetTranslator::translate, 
> and even learn about using the template keyword for disambiguation. 
> Eventually there was enough information to conclude that yes, it probably is 
> safe since the ListHashSetNodes are allocated on the heap by 
> ListHashSetTranslator::translate, so even though the HashTable invalidates 
> its own iterators and HashTable::rehash may reallocate its storage, the 
> actual ListHashSetNode pointed to by ListHashSetConstInterator should 
> continue to exist. But constantly having to do such deep research makes 
> coding highly inefficient, and there's a high risk of making errors.

I think documenting the public interfaces of core data structures is probably a 
worthwhile tradeoff, since they change rarely but are used in many places. In 
fact, ListHashSet already has a class comment and documents some method 
behavior that is not obvious from the signature. Covering the issue you mention 
seems worthwhile as well.

https://bugs.webkit.org/show_bug.cgi?id=91106

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Alan Stearns]

The spec itself consistently and deliberately calls them "CSS Regions," so a 
CSS prefix could be appropriate.

Thanks,

Alan


From: Adam Barth mailto:aba...@webkit.org>>
To: Andrei Bucur mailto:abu...@adobe.com>>
Cc: "webkit-dev@lists.webkit.org" 
mailto:webkit-dev@lists.webkit.org>>
Subject: Re: [webkit-dev] Web APIs and class name collisions

One common thing we do is prefix "DOM" to DOM-level concepts.  For example, 
DOMWindow and DOMFileSystem.  I'm not sure if we have an established convention 
for CSS-level concepts.

Adam


On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur 
mailto:abu...@adobe.com>> wrote:
Hello Webkittens!

While implementing the Region interface ( 
http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed that 
the name "Region" is already taken by a class in platform/graphics. I'd like to 
know what's the best approach in these kind of situations:

 1.  Rename the existing WebCore class to something else and use the name 
"Region" for the Web API so there's parity between the implementation and the 
spec
 2.  Somehow prefix the Web API implementation class name?

As the Web APIs expand I suppose this situation may occur again in the future 
and I suppose there should be a rule describing what's the best approach to 
take.

Thanks!
Andrei.

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Web APIs and class name collisions

[Adam Barth]

One common thing we do is prefix "DOM" to DOM-level concepts.  For example,
DOMWindow and DOMFileSystem.  I'm not sure if we have an established
convention for CSS-level concepts.

Adam


On Thu, Jul 12, 2012 at 9:18 AM, Andrei Bucur  wrote:

> Hello Webkittens!
>
> While implementing the Region interface (
> http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed
> that the name "Region" is already taken by a class in platform/graphics.
> I'd like to know what's the best approach in these kind of situations:
>
>1. Rename the existing WebCore class to something else and use the
>name "Region" for the Web API so there's parity between the implementation
>and the spec
>2. Somehow prefix the Web API implementation class name?
>
> As the Web APIs expand I suppose this situation may occur again in the
> future and I suppose there should be a rule describing what's the best
> approach to take.
>
> Thanks!
> Andrei.
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Web APIs and class name collisions

[Andrei Bucur]

Hello Webkittens!

While implementing the Region interface ( 
http://dev.w3.org/csswg/css3-regions/#the-region-interface ) I've noticed that 
the name "Region" is already taken by a class in platform/graphics. I'd like to 
know what's the best approach in these kind of situations:

 1.  Rename the existing WebCore class to something else and use the name 
"Region" for the Web API so there's parity between the implementation and the 
spec
 2.  Somehow prefix the Web API implementation class name?

As the Web APIs expand I suppose this situation may occur again in the future 
and I suppose there should be a rule describing what's the best approach to 
take.

Thanks!
Andrei.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

[John Mellor]

On Wed, Jul 11, 2012 at 4:53 PM, Ryosuke Niwa  wrote:

>
> On Jul 11, 2012 8:43 AM, "John Mellor"  wrote:
> >
> > On Wed, Jul 11, 2012 at 4:21 PM, Ryosuke Niwa  wrote:
> >>
> >> What's the point of adding this comment when the URL contains all the
> information?  All we need is the URL.  If anything, we should be describing
> the difference between the inline boxes in CSS2.1 and our implementation
> instead.
> >
> > That would be great! I agree that there's probably limited value in just
> copy/pasting from specs like I did. Linking to the spec something is based
> on and describing the differences would add a lot of value.
>
> The problem is that we'll then incur the maintenace cost of keeping
> comments up-to-date and the risk of them getting out-of-date as we have
> previously discussed.
>
> - Ryosuke
>
Like many others on this thread, I don't buy this argument. It's well known
that developers spend much more time reading & understanding code than
writing it. This is especially true in an inevitably complex codebase like
WebKit, with many interdependencies. Well chosen comments that add value
(like describing the deviations from a spec) are worth maintaining. Indeed,
12 months down the line, the original author will probably have forgotten
enough detail that they themselves would have benefited by appropriately
commenting their code.

Even to check simple things like whether some method can return null you
typically have to grep through all references to the thing being returned,
and often in turn through all references to those. It's not feasible for
developers to read the full transitive closure of code that interacts with
every class they use, so they skim it and make mistakes. These mistakes
often then slip through code review because they look reasonable. The bugs
caused by these incorrect assumptions (that wouldn't have occurred with
appropriate comments) seem much more concerning than the hypothetical bugs
caused by stale comments.

To take an arbitrary example, lets say that while iterating through a
ListHashSet something causes entries to be deleted. Intuitively it seems
this needn't invalidate the iterator, as long as the entry the iterator is
currently pointing to isn't removed. But is that actually the case in this
particular implementation? A well-documented library like Java's
LinkedHashSet
will
warn you "if the set is modified at any time after the iterator is created,
in any way except through the iterator's own remove method, the iterator
will throw a ConcurrentModificationException" and that's that. I just tried
to find this out in WebKit and had to read though ListHashSetIterator,
ListHashSetConstIterator, ListHashSetNode, ListHashSet::remove,
ListHashSet::unlinkAndDelete, HashTable::remove,
HashTable::internalCheckTableConsistency,
HashTable::removeWithoutEntryConsistencyCheck,
HashTable::removeAndInvalidateWithoutEntryConsistencyCheck,
HashTable::invalidateIterators, HashTable::shrink, HashTable::rehash,
ListHashSet::add, HashTable::add,
HashTable::makeKnownGoodIterator, HashTableIterator, HashTable::AddResult
and ListHashSetTranslator::translate, and even learn about using the template
keyword
for
disambiguation. Eventually there was enough information to conclude that
yes, it probably is safe since the ListHashSetNodes are allocated on the
heap by ListHashSetTranslator::translate, so even though the HashTable
invalidates its own iterators and HashTable::rehash may reallocate its
storage, the actual ListHashSetNode pointed to by ListHashSetConstInterator
should continue to exist. But constantly having to do such deep research
makes coding highly inefficient, and there's a high risk of making errors.

All the best,
John
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev