On Tue, Aug 29, 2017 at 2:14 PM, Keith Miller <keith_mil...@apple.com> wrote: > > >> 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.
As Darin suggested, <Class>Internal is appropriate for files like Element.cpp, Node.cpp, etc... which contains definitions of a class. There's an existing convention in WebKit that implementation details are suffixed by "Internal". It's probably more appropriate to use <Filename>Static for files like Editing.cpp, which is a collection of a bunch of global functions. > 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. Makes sense. - R. Niwa _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
