> 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 firstname.lastname@example.org https://lists.webkit.org/mailman/listinfo/webkit-dev