Re: [webkit-dev] [webkit-changes] [57262] trunk/JavaScriptCore
Hi, Alexey Proskuryakov írta: > "FIXME! " is different from "FIXME: " in that Xcode doesn't recognize > it. I'm surprised that style guide doesn't say anything about FIXME vs. > TODO. I wasn't aware of this, thanks for your advice, I will use "FIXME:" next time. > + // [Qt]r57240 broke Qt build (might be a gcc bug) > + // FIXME! See: https://bugs.webkit.org/show_bug.cgi?id=37253 > But I'm not sure if a comment was even needed here - the ugliness of > nested #ifs shouts the same. This patch is only a workaround for buggy gcc. I added this comments, because I want to avoid that somebody would like to optimize Qt port and remove these guards. Ugliness of nested #ifs is another question, I hate them as you. It would be great if we can define it in Coding Style Guidelines. We can found different styles for nested #ifdefs in trunk (for example in JavaScriptCore/wtf/Platform.h( style-I.) #if xxx #if yyy ... #else ... #endif #endif style-II.) #if xxx #if yyy ... #else ... #endif // yyy #endif // xxx style-III.) #if xxx #if yyy ... #else ... #endif // yyy #endif // xxx style-IV.) #if xxx # if yyy ... # else ... # endif #endif As for me, I prefer style-III, but all reviewer ask me to modify my patches into style-I or style-II. I think we should make consensus which of them is the preferred coding style. br, Ossy ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] [webkit-changes] [57262] trunk/JavaScriptCore
On Apr 8, 2010, at 11:18 AM, Alexey Proskuryakov wrote: On 08.04.2010, at 10:21, Jeremy Orlow wrote: I wasn't even aware that Xcode did recognize it or that we used that convention because it does. We should probably document this somewhere. I don't know if that's the original or only reason. Just something I noticed on my own at some point, and thought it was a sufficiently good explanation. I'm surprised that style guide doesn't say anything about FIXME vs. TODO. What do you mean? Are you suggesting that we should be using both and for different purposes? Sorry for being unclear. I meant that coding style guidelines should document always using FIXME. That seems like a good change to the guidelines. Specifically "FIXME: " as a prefix, with discouragement of TODO or XXX or other such formats. - Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] [webkit-changes] [57262] trunk/JavaScriptCore
On 08.04.2010, at 10:21, Jeremy Orlow wrote: I wasn't even aware that Xcode did recognize it or that we used that convention because it does. We should probably document this somewhere. I don't know if that's the original or only reason. Just something I noticed on my own at some point, and thought it was a sufficiently good explanation. I'm surprised that style guide doesn't say anything about FIXME vs. TODO. What do you mean? Are you suggesting that we should be using both and for different purposes? Sorry for being unclear. I meant that coding style guidelines should document always using FIXME. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] [webkit-changes] [57262] trunk/JavaScriptCore
On Thu, Apr 8, 2010 at 5:59 PM, Alexey Proskuryakov wrote: > > On 08.04.2010, at 1:16, o...@webkit.org wrote: > > + // [Qt]r57240 broke Qt build (might be a gcc bug) > > + // FIXME! See: https://bugs.webkit.org/show_bug.cgi?id=37253 > > > "FIXME! " is different from "FIXME: " in that Xcode doesn't recognize it. > I wasn't even aware that Xcode did recognize it or that we used that convention because it does. We should probably document this somewhere. > I'm surprised that style guide doesn't say anything about FIXME vs. TODO. > What do you mean? Are you suggesting that we should be using both and for different purposes? > But I'm not sure if a comment was even needed here - the ugliness of nested > #ifs shouts the same. > > - WBR, Alexey Proskuryakov > > > ___ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > > ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] [webkit-changes] [57262] trunk/JavaScriptCore
On 08.04.2010, at 1:16, o...@webkit.org wrote: +// [Qt]r57240 broke Qt build (might be a gcc bug) +// FIXME! See: https://bugs.webkit.org/show_bug.cgi?id=37253 "FIXME! " is different from "FIXME: " in that Xcode doesn't recognize it. I'm surprised that style guide doesn't say anything about FIXME vs. TODO. But I'm not sure if a comment was even needed here - the ugliness of nested #ifs shouts the same. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
