donejs / cli

The DoneJS command line interface
https://www.npmjs.com/package/donejs-cli
MIT License
6 stars 7 forks source link

Alphabetize done-serve in dependency list #82

Closed leoj3n closed 7 years ago

leoj3n commented 7 years ago

Fixes donejs/generator-donejs#219

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 93.103% when pulling 5f499d14ea0b0b200a8d3487c919adf56d2acd47 on alphabetize-done-serve into 69030c14d65bb017ee041dc92d3fbe2128b5700a on master.

chasenlehara commented 7 years ago

Hi @leoj3n, thanks for submitting this. As far as I know, this PR doesn’t fix https://github.com/donejs/generator-donejs/issues/219 because that issue is related to the generated output of the CLI, not the CLI’s dependencies. I’ve reopened it because the issue needs to be fixed in that project, not this one.

leoj3n commented 7 years ago

It can be proven that this PR solves donejs/generator-donejs#219 like so:

$ mkdir workon-donejs-cli && cd workon-donejs-cli
$ git clone https://github.com/donejs/donejs.git
$ git clone https://github.com/donejs/cli.git
$ cd donejs
$ git checkout develop && npm install && npm link

Edit lib/cli/cmd-init.js:

// install donejs-cli
function installCli(version, options) {
  var pkg = 'donejs-cli@' + utils.versionRange(version);
  console.log('INJECTING LOCAL COPY OF donejs-cli');
  var pkg = '../../../cli';
  var npmArgs = [ 'install', pkg, '--loglevel', 'error' ];
  return utils.spawn('npm', npmArgs, options);
}
$ cd ../cli

Edit package.json to replace the packages defined in donejs.dependencies with:

  "donejs": {
    "dependencies": {
      "random": "*"
    },
$ cd .. && mkdir fake-app && cd fake-app
$ donejs add app # accept defaults for all prompts
$ npm test

Now, check package.json; it should have dependencies like:

  "dependencies": {
    "random": "*"
  },

Thus, proving that this PR does affect the generated app's package.json!

chasenlehara commented 7 years ago

Cool, thanks for testing it and correcting me.