> On Aug 29, 2017, at 11:37 AM, Maciej Stachowiak <m...@apple.com> wrote:
> 
> 
> 
>> On Aug 29, 2017, at 11:31 AM, Darin Adler <da...@apple.com> wrote:
>> 
>> Sent from my iPhone
>> 
>>> On Aug 29, 2017, at 11:22 AM, Keith Miller <keith_mil...@apple.com> wrote:
>>> 
>>> I doubt anyone is going to run such a script before they go to upload a 
>>> patch to bugzilla. 
>> 
>> EWS was what I was hoping for; likely to be sufficient. But it could also be 
>> integrated into the development process as, say, check-webkit-style is.
> 
> check-webkit-style is run by both EWS and webkit-patch upload, in addition to 
> being hand-runnable, so that seems like a good place to put this new kind of 
> check.

I agree and my intention was to add the check to check-webkit-style.

> 
>> 
>>> So developers will still hit the name collision issue randomly throughout 
>>> development.
>> 
>> Sure.
>> 
>> But I don’t think that required extensive use of namespaces is the best way 
>> to greatly mitigate this. Mistakes will still happen. So I think we 
>> shouldn’t go too far in ruining readability of code for something that is 
>> not necessary to solve the problem.
>> 
>> Recommending either namespaces or globally unique names and clarifying that 
>> file local scope doesn’t exist are both good.
>> 
>> But again I think people already handle these problems fine in headers so we 
>> don’t need too tight a straitjacket, at least not out of the gate.
> 
> I tend to agree with this. I think keeping names of static functions globally 
> unique is reasonable, so long as we have an automated way to check. This 
> seems better than namespaces. With namespaces, it's still possible to make a 
> mistake, such as by having a using at global scope, so we'd need the style 
> checker to enforce some kind of rule.
> 
> If we were to use namespaces, then properly naming them seems better than the 
> FILENAME macro.

I’ll defer to your judgement on properly naming the namespaces over using the 
macro. Does anyone have strong opinions on what rules the names should follow? 
I think Darin suggested <Filename>Internal. I think I would prefer 
<Filename>Static as that’s very clear what the namespace represents. I think I 
have been convinced that, for the most part we shouldn’t need to have such a 
strict rule on naming collisions. There are probably places where it makes more 
sense to have the namespace and places where it doesn’t. 

Here’s my proposal for the style checker. It should require that there are no 
duplicate globally visible variables in a given directory. We don’t need to 
worry about different directories since those files can’t be included in the 
same bundle under my proposal. While, it could be inconvenient for new 
developers it should keep the code much cleaner. Additionally, we can have a 
bot that builds with full per directory includes to ensure that we don’t have 
any collisions. As an aside, we might also want a bot that builds without 
unified sources to ensure people don’t rely on headers that happen to be above 
them in the bundle.

Does this seem reasonable?

> 
> Regards,
> Maciej

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to