| Summary: | Progress towards building AppleWin with CMake | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||
| Component: | WebKit Misc. | Assignee: | Alex Christensen <achristensen> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | ||||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Alex Christensen
2015-07-27 11:04:20 PDT
Created attachment 257568 [details]
Patch
Comment on attachment 257568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257568&action=review Looks pretty reasonable to me, but I have a question about the new platform-independent include path. > Source/JavaScriptCore/CMakeLists.txt:36 > "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector" > + "${CMAKE_BINARY_DIR}/Source/JavaScriptCore" What files are generated here? It might be more appropriate to generate them in another directory or to move this include to the platform-specific CMake file. I ask because the GTK+ port uses Ninja and this include is not necessary for it. > Source/WebCore/PlatformAppleWin.cmake:12 > +) > +list(APPEND WebCore_SOURCES Style nit. I think there is usually an empty line of whitespace between definitions. (In reply to comment #2) > Comment on attachment 257568 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257568&action=review > > Looks pretty reasonable to me, but I have a question about the new > platform-independent include path. > > > Source/JavaScriptCore/CMakeLists.txt:36 > > "${DERIVED_SOURCES_JAVASCRIPTCORE_DIR}/inspector" > > + "${CMAKE_BINARY_DIR}/Source/JavaScriptCore" > > What files are generated here? It might be more appropriate to generate them > in another directory or to move this include to the platform-specific CMake > file. I ask because the GTK+ port uses Ninja and this include is not > necessary for it. It was something that's #included in LowLevelInterpreterWin.asm. I should probably just generate that into a different directory somehow. r=you without this change and the style nit? Created attachment 257597 [details]
Patch
Comment on attachment 257597 [details]
Patch
Thanks!
|