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. |
DescriptionIssue 2271 - Enable documentation for Chrome extensions
Patch Set 1 : #
Total comments: 5
MessagesTotal messages: 6
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 Maybe instead of this logic we could use a JSdoc configuration file with an exclude pattern? http://usejsdoc.org/about-configuring-jsdoc.html
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 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.
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: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.
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: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.
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. 
|
