Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5960911785295872: Issue 2271 - Enable documentation for Chrome extensions (Closed)

Created:
April 4, 2015, 4:40 p.m. by Sebastian Noack
Modified:
April 10, 2015, 6:30 a.m.
Reviewers:
kzar
CC:
Wladimir Palant
Visibility:
Public.

Description

Issue 2271 - Enable documentation for Chrome extensions

Patch Set 1 : #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -5 lines) Patch
M build.py View 2 chunks +9 lines, -5 lines 5 comments Download

Messages

Total messages: 6
Sebastian Noack
April 4, 2015, 4:40 p.m. (2015-04-04 16:40:54 UTC) #1
kzar
http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py File build.py (right): http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py#newcode375 build.py:375: # JSDoc struggles wih huge objects: https://github.com/jsdoc3/jsdoc/issues/976 Maybe instead ...
April 5, 2015, 12:51 p.m. (2015-04-05 12:51:41 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py File build.py (right): http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py#newcode375 build.py:375: # JSDoc struggles wih huge objects: https://github.com/jsdoc3/jsdoc/issues/976 On 2015/04/05 ...
April 5, 2015, 1:08 p.m. (2015-04-05 13:08:56 UTC) #3
kzar
http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py File build.py (right): http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py#newcode375 build.py:375: # JSDoc struggles wih huge objects: https://github.com/jsdoc3/jsdoc/issues/976 On 2015/04/05 ...
April 5, 2015, 1:30 p.m. (2015-04-05 13:30:51 UTC) #4
Sebastian Noack
http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py File build.py (right): http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/build.py#newcode375 build.py:375: # JSDoc struggles wih huge objects: https://github.com/jsdoc3/jsdoc/issues/976 On 2015/04/05 ...
April 5, 2015, 1:34 p.m. (2015-04-05 13:34:33 UTC) #5
kzar
April 5, 2015, 3:12 p.m. (2015-04-05 15:12:55 UTC) #6
LGTM

http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/buil...
File build.py (right):

http://codereview.adblockplus.org/5960911785295872/diff/5724160613416960/buil...
build.py:375: # JSDoc struggles wih huge objects:
https://github.com/jsdoc3/jsdoc/issues/976
On 2015/04/05 13:34:33, Sebastian Noack wrote:
> On 2015/04/05 13:30:51, kzar wrote:
> > On 2015/04/05 13:08:56, Sebastian Noack wrote:
> > > On 2015/04/05 12:51:41, kzar wrote:
> > > > Maybe instead of this logic we could use a JSdoc configuration file with
> an
> > > > exclude pattern? http://usejsdoc.org/about-configuring-jsdoc.html
> > > 
> > > This would be kinda nice, because the JSDoc configuration could be done in
> the
> > > repective repo then, and the logic in buildtools could be agnostic of the
> > source
> > > files.
> > > 
> > > But on the other hand this hack will (hopefully) be just temporarily. And
> the
> > > logic is simpler without delegating to a config file.
> > 
> > Wouldn't the logic be pretty simple if we use a config file though? I
thought
> we
> > could just add an extra argument to the command array to specify the config
> > path, and then just use the configuration for special things like excluding
> > publicSuffixList.js. Otherwise avoiding changes to this chunk of code.
> 
> The logic here would be reasonable. But the actual logic is just delegated to
> the configfile. And spreading logic isn't great, unless there is a reason for
> it.

Fair enough I suppose.

Powered by Google App Engine
This is Rietveld