Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(580)

Issue 237443004: pkg/docgen: removed yaml and append output support (Closed)

Created:
6 years, 8 months ago by kevmoo
Modified:
6 years, 8 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

pkg/docgen: removed yaml and append output support R=efortuna@google.com Committed: https://code.google.com/p/dart/source/detail?r=35039

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -159 lines) Patch
M pkg/docgen/bin/docgen.dart View 3 chunks +0 lines, -10 lines 0 comments Download
M pkg/docgen/lib/docgen.dart View 2 chunks +3 lines, -4 lines 0 comments Download
D pkg/docgen/lib/src/dart2yaml.dart View 1 chunk +0 lines, -84 lines 0 comments Download
M pkg/docgen/lib/src/generator.dart View 10 chunks +23 lines, -59 lines 0 comments Download
M utils/apidoc/docgen.gyp View 1 chunk +1 line, -2 lines 3 comments Download

Messages

Total messages: 4 (0 generated)
kevmoo
Will do a separate CL (with tests) for pretty output
6 years, 8 months ago (2014-04-14 19:03:32 UTC) #1
Emily Fortuna
lgtm
6 years, 8 months ago (2014-04-14 20:53:47 UTC) #2
kevmoo
Committed patchset #1 manually as r35039 (presubmit successful).
6 years, 8 months ago (2014-04-14 20:56:21 UTC) #3
ricow1
6 years, 8 months ago (2014-04-23 07:16:07 UTC) #4
Message was sent while issue was closed.
Kevin: Sorry to hijack your cl, but it just caught my attention due to:
https://code.google.com/p/dart/issues/detail?id=18392
Just one comma comment for you :-)
Alan: I think this was already here from the beginning on, if I reviewed this
and missed it I apologize

https://codereview.chromium.org/237443004/diff/1/utils/apidoc/docgen.gyp
File utils/apidoc/docgen.gyp (right):

https://codereview.chromium.org/237443004/diff/1/utils/apidoc/docgen.gyp#newc...
utils/apidoc/docgen.gyp:67: '../../tools/only_in_release_mode.py',
it seems that we are using ../../pkg below as an argument to docgen, if we are
actually using ../../pkg we should have the files as input here. You can either
do that by doing list_files.py on ../../pkg or better, since you will only have
one more file, many inputs can mess up xcodebuild on mac: add a dependency on
../../pkg/pkg_files.gyp:pkg_files_stamp and in input have 
'<(SHARED_INTERMEDIATE_DIR)/pkg_files.stamp',

It may be that we have this "dependency implicitly through another
dependency/input - but we should not rely on it

https://codereview.chromium.org/237443004/diff/1/utils/apidoc/docgen.gyp#newc...
utils/apidoc/docgen.gyp:77: '../../sdk/bin/dart',
Here you should use:
 '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)dart<(EXECUTABLE_SUFFIX)',
since you are depending on this in the inputs list
also, using that line will fix
https://code.google.com/p/dart/issues/detail?id=18392

https://codereview.chromium.org/237443004/diff/1/utils/apidoc/docgen.gyp#newc...
utils/apidoc/docgen.gyp:87: '../../pkg'
it is pretty standard to have trailing commas in lists in python (we and
chromium have it all over our gyp files)

Powered by Google App Engine
This is Rietveld 408576698