Here's a wip for the breakout of the config parser module https://github.com/lorinbeer/cordova-lib/tree/configparser_module
still need to tidy up the referencing but feedback welcome On Fri, Jun 6, 2014 at 11:03 AM, Anis KADRI <[email protected]> wrote: > The goal is to have independent modules that can be consumed. > > Andrew, your second example reflects what we want to achieve. Obviously, > there will be more than CordovaError. Brian and I think that we can keep > the current cordova-lib repository and create folders for each module (each > with its own dependencies and package.json). Eventually, there could be > more repositories if we chose to have them. > > Right now if you just want one piece of functionality from either plugman > or cli you need the whole cordova-lib module which is extremely unfortunate > and pretty silly. > > > On Fri, Jun 6, 2014 at 10:55 AM, Michal Mocny <[email protected]> wrote: > >> Additionally, I think it will only be a problem when each of the modules >> call into one another. I.e. when cordova-cli calls into cordova-plugman >> and that throws a CordovaError which is caught by cordova-cli and then >> compared with instanceof to its own version.. Its not a problem in >> isolation. >> >> This problem usually comes up with singletons / internal caching, such as >> reading config files from disk once and then modifying in-memory, but I'm >> not sure if we actyally have that problem. >> >> -Michal >> >> >> On Fri, Jun 6, 2014 at 1:12 PM, Andrew Grieve <[email protected]> >> wrote: >> >> > On Fri, Jun 6, 2014 at 12:37 PM, Brian LeRoux <[email protected]> wrote: >> > >> > > Ya I spoke to Issac a bit about this last week and he wasn't being >> > entirely >> > > facetious. One path that apparently works well is to treat root >> > > `./node_modules` as something npm manages and then nested node_modules >> > for >> > > userspace stuff. >> > > >> > > eg. `./src/node_modules` and then `var foo = require('src/foo')` >> > > >> > > Not a huge win over just using relative paths so meh. >> > > >> > > I don't understand why instanceof wouldn't work? >> > > >> > >> > You probably have a better understanding of this than I do... But: >> > - There's no problem when running locally, since we've defined the >> > structure of node_modules to work, >> > >> > But when npm install is run on an end-users machine, if it ends up like: >> > >> > cordova-lib/node_modules/cordova-plugman/node_modules/CordovaError >> > cordova-lib/node_modules/cordova-cli/node_modules/CordovaError >> > cordova-lib/node_modules/CordovaError >> > >> > then there are now 3 CordovaError constructors, so instanceof will not >> work >> > between them. >> > >> > Now, if npm sees that all packages call for the same version of >> > CordovaError, and install them like: >> > >> > cordova-lib/node_modules/cordova-plugman >> > cordova-lib/node_modules/cordova-cli >> > cordova-lib/node_modules/CordovaError >> > >> > then all is well. >> > >> > Easy to answer with a quick test :) >> > >> > >> > >> > > >> > > >> > > On Fri, Jun 6, 2014 at 9:14 AM, Andrew Grieve <[email protected]> >> > > wrote: >> > > >> > > > Exciting! I tried the "check in node_modules" approach in >> > > > cordova-app-harness and it works really well there: >> > > > >> https://github.com/apache/cordova-app-harness/tree/master/harness-push >> > > > It's a simpler case since there's a strict parent/child relationship. >> > > > >> > > > The main unknown for me is how to make CordovaError work as its own >> > > module >> > > > since we'll require that there is never multiple copies of the module >> > > when >> > > > the end-user types "npm install" (or else the instanceof check won't >> > > work). >> > > > I *think* that npm does the right thing so long as all of the >> packages >> > > use >> > > > the same version constraints for it, but I haven't tested it. >> > > > >> > > > >> > > > On Fri, Jun 6, 2014 at 11:37 AM, Michal Mocny <[email protected]> >> > > wrote: >> > > > >> > > > > Brian: sounds good. Looking forward to see where we get. >> > > > > >> > > > > -Michal >> > > > > >> > > > > >> > > > > On Fri, Jun 6, 2014 at 11:36 AM, Brian LeRoux <[email protected]> wrote: >> > > > > >> > > > > > @Michal: we're going to start extracting things to their own >> > modules. >> > > > (As >> > > > > > discussed.) We're going to start small and simple: CordovaError, >> > > > > > SuperSpawn, etc. But before we do that we'll try to merge up as >> > many >> > > > PRs >> > > > > as >> > > > > > possible. Ideally there are none when we get rolling. (Next >> week.) >> > > > > > >> > > > > > Many repos (or just small modules) would isolate the changes >> making >> > > it >> > > > > > easier to refactor. Anyhow: I'm tired of advocating that design >> > > pattern >> > > > > > choice and I'm sure you're tired of hearing about it! >> > > > > > >> > > > > > >> > > > > > On Thu, Jun 5, 2014 at 6:04 PM, Michal Mocny < >> [email protected]> >> > > > > wrote: >> > > > > > >> > > > > > > On Thu, Jun 5, 2014 at 6:47 PM, Brian LeRoux <[email protected]> >> wrote: >> > > > > > > >> > > > > > > > we're about to do some heavy refactoring on cordova-lib…it >> > would >> > > > > appear >> > > > > > > > many ppl are working on it atm: >> > > > > > > > >> > > > > > > >> > > > > > > Who/how/what/why? >> > > > > > > >> > > > > > > >> > > > > > > > >> > > > > > > > https://github.com/apache/cordova-lib/pulls >> > > > > > > > >> > > > > > > > I'm afraid to find out how many *aren't* open PR's …but this >> > > > > certainly >> > > > > > > > illustrates the problem with one giant repo with all our code >> > > > > > > > >> > > > > > > >> > > > > > > If you are refactoring the code in a way that breaks PR from >> > > > applying, >> > > > > I >> > > > > > > don't think multiple repos would make a difference. Probably >> > this >> > > > way >> > > > > is >> > > > > > > easier if you were making changes that needed to touch multiple >> > > > repos. >> > > > > > But >> > > > > > > this conversation feels sorta Deja Vu. >> > > > > > > >> > > > > > > -Michal >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >>
