I'm happy leaving it to you -- but these days I've been creating pretty fine-grained features. It's really just a matter of convenience vs. optimization in JS bundles.
On Tue, Apr 26, 2011 at 4:09 PM, <[email protected]> wrote: > Thanks for the comments John. I am fine with moving this functionality > out of the core code, but I am debating where it should go....should I > create a completely new feature or maybe we could add it to the xmlutil > feature. Paul had mentioned that the xmlutil feature is used for > templating code. I just didn't want to create a whole new feature just > for one simple function, unless everyone agrees thats the best thing to > do. Thoughts? > > > On 2011/04/26 01:14:49, johnfargo wrote: > >> thoughts welcome. >> > > > > http://codereview.appspot.com/4438071/diff/1/features/src/main/javascript/features/core.json/feature.xml > >> File features/src/main/javascript/features/core.json/feature.xml >> > (right): > > > > http://codereview.appspot.com/4438071/diff/1/features/src/main/javascript/features/core.json/feature.xml#newcode33 > >> features/src/main/javascript/features/core.json/feature.xml:33: >> > <exports > >> type="js">gadgets.json.convertXmlToJson</exports> >> since there are no deps between this file and the rest of >> > gadgets.json, let's > >> split this off into a subfeature. Suggestion: gadgets.json.xml. Doing >> > so will > >> avoid putting additional size burden on those who want only to load >> > the "core" > >> JSON methods themselves. >> > > > > http://codereview.appspot.com/4438071/diff/1/features/src/main/javascript/features/core.json/json-xmltojson.js > >> File features/src/main/javascript/features/core.json/json-xmltojson.js >> > (right): > > > > http://codereview.appspot.com/4438071/diff/1/features/src/main/javascript/features/core.json/json-xmltojson.js#newcode32 > >> features/src/main/javascript/features/core.json/json-xmltojson.js:32: >> > //Integer > >> which represents a text node >> I'm getting some spacing issues here -- tabs perhaps rather than >> > 2-space indent? > > > > http://codereview.appspot.com/4438071/ >
