Gyp files for Cocoa shell and Debugger

[yangsu, 2011-06-16 20:41:09]

Here are the gyp files i wrote for my cocoa sample app and cocoa debugger app

[epoger, 2011-06-16 21:12:32]

Nice!  A few minor suggestions below.  Feel free to grab me to discuss any of
them.

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp
File gyp/CocoaDebuggerApp.gyp (right):

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode34
gyp/CocoaDebuggerApp.gyp:34: '../experimental/CocoaDebugger/SkDebugDumper.cpp',
please either alphabetize the sources, or put a comment at the top of each block
indicating why the grouping is meaningful

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode39
gyp/CocoaDebuggerApp.gyp:39: '../src/core/SkGlobals.cpp',
so does it not work unless you explicitly specify the files within src/core,
src/xml, and src/views?  I had hoped that the dependencies below would provide
all that's needed.

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode49
gyp/CocoaDebuggerApp.gyp:49: 'sources!' : [
delete this empty section

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode60
gyp/CocoaDebuggerApp.gyp:60: [ 'OS == "linux" or OS == "freebsd" or OS ==
"openbsd" or OS == "solaris"', {
please delete the empty unix and windows conditionals. if you want to, you could
add a comment saying "only supports mac for now"

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode80
gyp/CocoaDebuggerApp.gyp:80: 'libraries!': [
please make a note somewhere that in a future CL we should delete the default
dependency on Carbon and AGL.

http://codereview.appspot.com/4632045/diff/1/gyp/SimpleCocoaApp.gyp
File gyp/SimpleCocoaApp.gyp (right):

http://codereview.appspot.com/4632045/diff/1/gyp/SimpleCocoaApp.gyp#newcode27
gyp/SimpleCocoaApp.gyp:27: '../src/core/SkGlobals.cpp',
again, are the dependencies below not enough?

http://codereview.appspot.com/4632045/diff/1/gyp/SimpleCocoaApp.gyp#newcode37
gyp/SimpleCocoaApp.gyp:37: 'sources!' : [
please delete empty sections throughout

[yangsu, 2011-06-16 22:12:08]

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp
File gyp/CocoaDebuggerApp.gyp (right):

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode34
gyp/CocoaDebuggerApp.gyp:34: '../experimental/CocoaDebugger/SkDebugDumper.cpp',
On 2011/06/16 21:12:32, epoger wrote:
> please either alphabetize the sources, or put a comment at the top of each
block
> indicating why the grouping is meaningful

Done.

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode39
gyp/CocoaDebuggerApp.gyp:39: '../src/core/SkGlobals.cpp',
On 2011/06/16 21:12:32, epoger wrote:
> so does it not work unless you explicitly specify the files within src/core,
> src/xml, and src/views?  I had hoped that the dependencies below would provide
> all that's needed.

Done.

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode49
gyp/CocoaDebuggerApp.gyp:49: 'sources!' : [
On 2011/06/16 21:12:32, epoger wrote:
> delete this empty section

Done.

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode60
gyp/CocoaDebuggerApp.gyp:60: [ 'OS == "linux" or OS == "freebsd" or OS ==
"openbsd" or OS == "solaris"', {
On 2011/06/16 21:12:32, epoger wrote:
> please delete the empty unix and windows conditionals. if you want to, you
could
> add a comment saying "only supports mac for now"

Done.

http://codereview.appspot.com/4632045/diff/1/gyp/CocoaDebuggerApp.gyp#newcode80
gyp/CocoaDebuggerApp.gyp:80: 'libraries!': [
On 2011/06/16 21:12:32, epoger wrote:
> please make a note somewhere that in a future CL we should delete the default
> dependency on Carbon and AGL.

Done.

http://codereview.appspot.com/4632045/diff/1/gyp/SimpleCocoaApp.gyp
File gyp/SimpleCocoaApp.gyp (right):

http://codereview.appspot.com/4632045/diff/1/gyp/SimpleCocoaApp.gyp#newcode27
gyp/SimpleCocoaApp.gyp:27: '../src/core/SkGlobals.cpp',
On 2011/06/16 21:12:32, epoger wrote:
> again, are the dependencies below not enough?

Done.

http://codereview.appspot.com/4632045/diff/1/gyp/SimpleCocoaApp.gyp#newcode37
gyp/SimpleCocoaApp.gyp:37: 'sources!' : [
On 2011/06/16 21:12:32, epoger wrote:
> please delete empty sections throughout

Done.

[epoger, 2011-06-17 14:16:55]

LGTM