Cool - nice change. You should take a quick look at pending CL 10107749. (Be sure to set 'Ignore Whitespace'.) It shows a more consistent way of adding a new third-party package. Most comments below are related to using this more consistent style.
======================================================================== http://mondrian.corp.google.com/file/10099212///depot/googleclient/gears/opensource/gears/Makefile?a=1 File //depot/googleclient/gears/opensource/gears/Makefile (snapshot 1) ------------------------------------ Line 372: ifneq ($(OFFICIAL_BUILD),1) For other third-party packages in this file, selective compilation is handled via a USING_SKIA define. You can then check for OFFICIAL_BUILD inside. (See the CL I referenced above.) ======================================================================== http://mondrian.corp.google.com/file/10099212///depot/googleclient/gears/opensource/gears/factory/factory_impl.cc?a=1 File //depot/googleclient/gears/opensource/gears/factory/factory_impl.cc (snapshot 1) ------------------------------------ Line 216: } else if (module_name == STRING16(L"beta.canvas")) { Weird indentation. Looks like you have an extra space? ======================================================================== http://mondrian.corp.google.com/file/10099212///depot/googleclient/gears/opensource/gears/tools/config.mk?a=1 File //depot/googleclient/gears/opensource/gears/tools/config.mk (snapshot 1) ------------------------------------ Line 169: CPPFLAGS += -I../third_party/skia/include/core -I../third_party/skia/include/images I'd change this to SKIA_CFLAGS, move it down alongside the other third-party CFLAGS, and then conditionally add it to CFLAGS and CPPFLAGS based on the value of USING_SKIA. (See similar code blocks for other third-party packages below.) ------------------------------------ Line 813: THIRD_PARTY_CPPFLAGS += /wd4244 /wd4800 As before, use SKIA_CFLAGS (and move between LIBGD_CFLAGS and SQLITE_CFLAGS). Then conditionally add it to COMPILE_FLAGS below, if USING_LLVM is set. ======================================================================== -- To respond, reply to this email or visit http://mondrian.corp.google.com/10099212
