See https://lists.webkit.org/pipermail/webkit-dev/2013-June/025056.html Better late than never.
Created attachment 362688 [details] Updates the style guideline
Comment on attachment 362688 [details] Updates the style guideline Looks fine
I'd wait a day or two to give a chance for people to respond on webkit-dev in the case they don't like this convention since the discussion happened 5 years ago.
Comment on attachment 362688 [details] Updates the style guideline View in context: https://bugs.webkit.org/attachment.cgi?id=362688&action=review > Websites/webkit.org/ChangeLog:8 > + Updating the coding style guidleline after the discussion following guideline
Comment on attachment 362688 [details] Updates the style guideline View in context: https://bugs.webkit.org/attachment.cgi?id=362688&action=review > Websites/webkit.org/code-style.md:686 > +```cpp > +StyleResolver* styleResolverIfExists(); > +StyleResolver& styleResolver(); > +``` First, if I were to vote today (02/21/2019), I would vote for StyleResolver& styleResolver() and StyleResolver* existingStyleResolver(), which was what Darin suggested when he wrote "I can’t remember what we decided on for our convention. I personally prefix the thing()/existingThing() pairing, but maybe I was outvoted?" in bug #194764, comment 18. Since Darin r+'ed this patch I guess he changed his mind. Funny thing though, if he didn't change his mind then his vote and mine would tip the scale towards: StyleResolver& styleResolver() and StyleResolver* existingStyleResolver() Funny, huh? I don't know how you came to think StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver() are the "right" choice, but Darin r+'ed it. So, I guess he warmed up to styleResolverIfExists() even though he started that webkit-dev thread you mentioned with "Lets bike shed! For some time, functions with names like **fooIfExists** and ensureFoo have been bothering me." I went through every single email in that thread (was a different time back made it clear what their position was with a "+1"). Here's what I found and it disagrees with your choices and I'm only mentioning it for completeness since Darin r+'ed the patch (and again he was the one that was initially unhappy). I did skip a few emails that just gave some other naming suggestions that nobody ever replied to as liking: Emil A Eklund eae at chromium.org: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025057.html> (BTW she no longer contributes) +1 StyleResolver* optionalStyleResolver() and StyleResolver& requiredStyleResolver() But suggested StyleResolver* optionalStyleResolver() and StyleResolver& styleResolver() (So to me this implies these are +1 by her as well) Geoffrey Garen ggaren at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025076.html> "I love this!" to Emil A Eklund's suggestion of StyleResolver* optionalStyleResolver() and StyleResolver& styleResolver() Ryosuke Niwa (You): <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025058.html> Suggest requireStyleResolver() Darin Adler: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025060.html> "Tentative +1" = 0.5 to Ryosuke's suggestion of requireStyleResolver(): "I’m warming to this idea. Maybe we can use “require” as a term of art, analogous to the way we use “create”, to mean “create if not already created”" Simon Fraser: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025061.html> -1 for requireStyleResolver(): "requireStyleResolver() sounds like it would return a bool." Maciej Stachowiak: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025061.html> Sounds like not in favor of requireStyleResolver(). Suggests to drop qualifier. Balazs Kelemen kbalazs at webkit.org: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025072.html> (BTW no longer contributes) Suggests styleResolverIfExists() and styleResolver() Timothy Hatcher timothy at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025073.html>: Suggests existingStyleResolver() and styleResolver(). Dan Bernstein mitz at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025073.html> "I like it." to Timothy Hatcher's suggestion. Filip Pizlo fpizlo at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025119.html> Likes styleResolverIfExists(): "That's why I like "fooIfExists"" Andreas Kling akling at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025074.html> (BTW no longer contributes) "+1 to these two." to Timothy Hatcher's suggestion of existingStyleResolver() and styleResolver(). Elliott Sprehn esprehn at chromium.org: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025080.html> (BTW no longer contributes) -1 to existingStyleResolver() and styleResolver(): "This doesn't make sense since calling styleResolver() again won't create a new one so it's also "existing style resolver"." +1 to foo() and ensureFoo() Maciej Stachowiak mjs at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025083.html> Disagrees with Elliott's suggestion. Suggests "fooIfExists/foo. But I agree with others on the thread that optionalFoo or existingFoo might be better names, particularly given std::optional" Gavin Barraclough barraclough at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-June/025070.html> -1 requireStyleResolver: "I find ‘requireStyleResolver()’ a little confusing." Suggests acquireStyleResolver(). My composite breakdown of the responses: Name for "guaranteed to give you a resolver" function: StyleResolver& styleResolver() => +8 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak) StyleResolver& requireStyleResolver() => +1.5 = +1 "Ryosuke Niwa" + 0.5 "Darin Adler" Name for function that returns, but does not create a resolver: StyleResolver* styleResolverIfExists() => +3 (Balazs Kelemen, Filip Pizlo, Maciej Stachowiak) StyleResolver* existingStyleResolver() => +3 (Timothy Hatcher, Dan Bernstein, Andreas Kling) StyleResolver* optionalStyleResolver() => +2 (Emil A Eklund, Geoffrey Garen) Adding in my votes and focusing on the winning results: StyleResolver& styleResolver() => +9 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak, Daniel Bates) StyleResolver* existingStyleResolver() => +4 (Timothy Hatcher, Dan Bernstein, Andreas Kling, Daniel Bates) Adding in Darin's votes if he changes his mind back to what he wrote in bug #194764, comment 18, final score is: StyleResolver& styleResolver() => +10 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak, Daniel Bates, Darin Adler) StyleResolver* existingStyleResolver() => +5 (Timothy Hatcher, Dan Bernstein, Andreas Kling, Daniel Bates, Darin Adler)
Darn. I missed some replies that spilled over to July: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025122.html>
Ryosuke Niwa (You): <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025122.html> +1 Balazs Kelemen's StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver(): "Maybe StyleResolver* styleResolverIfExists() StyleResolver& styleResolver()" Brady Eidson: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025123.html> +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver() Dan Bernstein mitz at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025124.html> +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver() Maciej Stachowiak mjs at apple.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025130.html> +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver() Bem Jones-Bey bjonesbe at adobe.com: <https://lists.webkit.org/pipermail/webkit-dev/2013-July/025132.html> (BTW no longer contributes) +1 StyleResolver* styleResolverIfExists() and StyleResolver& styleResolver() So, final tally for styleResolver() and styleResolverIfExists(), not including me or Darin: StyleResolver& styleResolver() => +10 (Emil A Eklund, Geoffrey Garen, Balazs Kelemen, Timothy Hatcher, Dan Bernstein, Andreas Kling, Elliott Sprehn, Maciej Stachowiak, Brady Eidson, Bem Jones-Bey) StyleResolver* styleResolverIfExists() => +7 (Balazs Kelemen, Filip Pizlo, Maciej Stachowiak, Ryosuke Niwa, Dan Bernstein, Brady Eidson, Bem Jones-Bey) So, even if I voted and Darin changed his mind on styleResolverIfExists(), styleResolverIfExists() would win. Okay. I'm fine with that.
Yeah, it was a pretty lengthy debate and there were many suggestions and up-votes. It was easier for me to go through because I had an email thread in my email archive. Thanks for summarizing the results though.
Committed r242273: <https://trac.webkit.org/changeset/242273>
<rdar://problem/48516506>