MathML code clean-up
Created attachment 120820 [details] Patch
Comment on attachment 120820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120820&action=review We don't usually land changes that only clean up style, unless they are really necessary for subsequent work. This just makes following svn blame harder. > Source/WebCore/rendering/mathml/RenderMathMLSubSup.cpp:215 > -} > +} // namespace WebCore This in particular is arguably a regression. There is no coding style rules that asks to comment closing braces, and I think that this just adds visual noise. If you suspect incorrect nesting, you are going to ask your editor to balance braces, not trust comments.
Alexey, thanks for your feedback (seriously). I do plan to do a lot of subsequent work in the rendering/mathml files, fixing a lot of bugs. I started by reporting and fixing several small bugs, and am now working on the first much larger one. I submitted this patch first though, following http://trac.webkit.org/wiki/CodeReview and http://trac.webkit.org/wiki/Creating%20and%20Submitting%20Layout%20Tests%20and%20Patches advice. I think my next patch will be easier to review if this one is done first. Perhaps I should create a bug for it now and say this bug blocks it? Any advice you can give me would be helpful, as I am new to WebKit. As for "} // namespace WebCore", I was just trying to make the rendering/mathml files more consistent. 72 of the 99 *.cpp files in WebKit/Source/WebCore/rendering/ contain this exact string. But I'm happy to skip this or other clean-up if it'd make it easier for other WebKit developers. It seems the important thing is to make subsequent major bug-fix patches easier to review.
P.S. I meant to say the "most important thing" was making review of further patches easier, because mathml needs many to be fixed and completed. I agree that svn blame is important also. It is true though that the mathml code is really still in an initial or prototype state, and will hopefully be changed a lot in the near-term.
I retract this patch, following Alexey's comments. This bug can be marked as resolved.
Comment on attachment 120820 [details] Patch Cleared review? from attachment 120820 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).