[webkit-reviews] review denied: [Bug 19743] Release build fails on Windows : [Attachment 29173] Combines html element cpp files into one whic helps reduce the size of webcore.lib

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied 's request for review:
Bug 19743: Release build fails on Windows
https://bugs.webkit.org/show_bug.cgi?id=19743

Attachment 29173: Combines html element cpp files into one whic helps reduce
the size of webcore.lib
https://bugs.webkit.org/attachment.cgi?id=29173&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 42150)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,21 @@
> +2009-04-01  Ben Murdoch  <benm at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Combines all the HTML element cpp files into one to help reduce the
size of WebCore.lib so we can build in release mode on 32bit Windows.
> +
> +	   * WebCore.vcproj/WebCore.vcproj:
> +	   * html/HTMLElements.cpp: Added.
> +
> +2009-04-01  Ben Murdoch  <benm at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Reduce the size of WebCore.lib so we can continue building release
mode on 32 bit Windows.
> +
> +	   * WebCore.vcproj/WebCore.vcproj:
> +	   * html/HTMLElements.cpp: Added.
> +

You have two ChangeLog entries here. It would be good to mention this bug in
your ChangeLog entry.

> @@ -12638,103 +12638,51 @@
>				>
>			</File>
>			<File
> -				RelativePath="..\html\HTMLAnchorElement.cpp"
> -				>
> -				<FileConfiguration
> -					Name="Release_PGO|Win32"
> -					>
> -					<Tool
> -						Name="VCCLCompilerTool"
> -						WholeProgramOptimization="true"

> -					/>
> -				</FileConfiguration>
> -			</File>
> -			<File

I think rather than removing the .cpp files from the project entirely (as was
unfortunately done for SVG), it would be better to use the "Excluded From
Build" option (right-click on the file, go to Configuration Properties >
General). That way the .cpp files will still be accessible from the Solution
Explorer. Eventually we should put the SVG .cpp files back into the .vcproj
using this method.

> Property changes on: WebCore/html/HTMLElements.cpp
> ___________________________________________________________________
> Added: svn:executable
>    + *

You should remove this property. We also normally set svn:eol-style to native
for new files.

Maybe the new file should have "AllInOne" in its name? That would match
JavaScriptCore's AllInOneFile.cpp and SVGAllInOne.cpp.

r- for now, but I don't think it will be much work to get this commitable!