Adds yam & config file #563

twokul · 2014-05-02T00:20:18Z

Closes #440

Adds support for .ember-cli settings file which looks like this:

{
  "port": 4200,
  "host": "0.0.0.0",
  "live-reload": true,
  "environment": "development"
}

By default, ember-cli is going to look for .ember-cli file in two places: the current directory and your home directory. You can put your personal settings in your home .ember-cli file, team shared settings should stay in project .ember-cli.

stefanpenner · 2014-05-02T00:24:43Z

great name 👍

stefanpenner · 2014-05-02T00:28:26Z

before we merge, some examples + ecli should utilize the config file. That way we know if this path is correct

twokul · 2014-05-02T00:48:37Z

@stefanpenner this a pre-PR for bringing analytics prompts back. I haven't really thought what we should put in .ember-cli except collectAnalytics = true/false.

rwjblue · 2014-05-02T00:50:03Z

@twokul Sure, but this doesn't do that yet, c/d? Meaning, lets see this thing work and have some tests to ensure the prompting + saving work in the context of ember-cli, then we can ...

twokul · 2014-05-02T00:51:33Z

It sets up the config file and reads/writes to it.

twokul · 2014-05-02T00:52:25Z

I can base prompt PR on this one.

MajorBreakfast · 2014-05-02T08:20:53Z

lib/models/project.js

  this.message = message;
-  this.stack = (new Error()).stack;
+  this.stack   = (new Error()).stack;
 }

 ProjectNotFoundError.constructor = ProjectNotFoundError;
 ProjectNotFoundError.prototype = new Error();


I think we should use Object.create(Error.prototype) there.

new Error() runs the constructor and I think we don't really want that

http://stackoverflow.com/questions/1382107/whats-a-good-way-to-extend-error-in-javascript#comment17126532_5251506 (comment)

Updated ARCHITECTURE.md to reflect that
https://github.com/stefanpenner/ember-cli/blob/master/ARCHITECTURE.md#custom-errors

a subject for a separate PR

fsmanuel · 2014-05-02T08:47:36Z

@twokul i would like to see proxy options and ci or silence in .ember-cli

twokul · 2014-05-02T12:30:30Z

@fsmanuel that's the plan

fsmanuel · 2014-05-02T16:27:00Z

@twokul 👍

stefanpenner · 2014-05-06T05:04:38Z

can you add support for the config file to have:

--proxy
--port
--blueprint

stefanpenner · 2014-05-24T01:33:30Z

lib/models/command.js

@@ -3,6 +3,7 @@
 var nopt          = require('nopt');
 var chalk         = require('chalk');
 var path          = require('path');
+var merge         = require('lodash-node').merge;


can you require the explicit module that contains merge?

lodash/objects/merge or something

stefanpenner · 2014-05-24T01:36:04Z

LGTM

stefanpenner · 2014-05-27T21:23:42Z

tests/helpers/fs-utils.js

+  }
+};
+
+var getUserHome = function getUserHome() {


var is not needed, the named function statement is sufficient.

stefanpenner · 2014-05-27T21:24:59Z

left some small comments, also this needs a rebase

rwjblue · 2014-06-01T03:09:04Z

Looks like this needs another rebase.

@stefanpenner - Anything else needed on this (other than the rebase)?

stefanpenner · 2014-06-01T03:58:42Z

tests/unit/settings-file/precedence-settings-test.js

+      'live-reload': true
+    });
+
+    touch(homePath, {


im abit concerned that this actually mutates my global config

paging @twokul

local settings in the project take precedence over your global settings. if local .ember-cli doesn't exists, your global settings will take over. does that answer your concern?

@twokul - The issue is that this test will modify the actual files in my home directory when running.

And if you look a few lines below, it will delete the file too. This means that I cannot have a ~/.ember-cli file while developing on the ember-cli project itself, but will have to manually recreate when I want to work on actual ember-cli applications (aka client projects).

@rjackson @stefanpenner sorry for confusion, I pushed the fix.

stefanpenner · 2014-06-05T04:49:55Z

this would be lovely to get in, it has been pending for a month. Can you please address the test concern?

rwjblue · 2014-06-06T19:21:22Z

@twokul - This is causing the fingerprinting smoke test to fail. Not sure yet exactly why, could you look into it?

twokul · 2014-06-06T19:24:26Z

@rjackson looking into it

andycrum · 2014-06-07T20:04:03Z

Anything new here? Would love to have this in master so I can use a couple of the things :)

stefanpenner · 2014-06-07T20:37:47Z

tests/unit/settings-file/precedence-settings-test.js

+      'live-reload': true
+    });
+
+    homeSettings = touchIfExists(homePath, {


can we not actually touch (or rely) peoples home dir during the tests? Simply configuration Yam with a path for home, via ./fixtures/home/. and ./fixtures/project/. would be best.

andycrum · 2014-06-08T05:36:26Z

@rjackson @twokul so the smoke test is failing because the test is calling build --environment=production, but it's actually generating the development environment. Changing blueprint/.ember-cli to production fixes it, strangely. In this case, maybe it's just failing because it's using an ember with an old blueprint to run the test?

rwjblue · 2014-06-08T12:02:40Z

The command line provided environment should win (meaning regardless of what was set in .ember-cli if an --environment option was specified it should be used).

Generally, I see the order of precedence (last wins):

Config file.
Environment variable.
Command line option.

This is essentially least specific to most specific.

andycrum · 2014-06-08T14:16:30Z

@rjackson yeah, the command line actually does win if you do a manual new and build. That's why this is so confusing. I'm not sure how the test is triggering the build that would cause it to use development

andycrum · 2014-06-08T21:05:27Z

Actually, the command line does not win. That is the issue (I had forgotten to npm link so I was running an older version of ember when trying the commands manually).

…take precedence over config

…g options

rwjblue · 2014-06-11T14:19:55Z

Looks like this has been updated to handle the various concerns that @stefanpenner and I pointed out. Probably ready for another review.

andycrum · 2014-06-11T14:20:36Z

Yay passing tests! Is this okay to be merged into master?

rwjblue · 2014-06-11T14:25:26Z

tests/unit/settings-file/precedence-settings-test.js

+      host:          '0.0.0.0'
+    });
+
+    settings = new Yam('ember-cli').getAll();


Can we modify to use the (recent) ability to mock the homePath with Yam? And just use a fixture in tests/fixtures to load the home settings (from above).

rwjblue · 2014-06-12T02:09:29Z

tests/helpers/fs-utils.js

@@ -0,0 +1,41 @@
+'use strict';


Looks like the recent changes (to mock yam) caused this to be an unneeded change.

Can you move this back to tests/helpers/file-utils.js (with the original contents)?

Adds yam & config file

stefanpenner · 2014-06-12T03:34:51Z

👍

rwjblue · 2014-06-12T03:34:53Z

BOOM

alvincrespo · 2014-06-13T13:39:27Z

👍 I'm stoked for this!

MajorBreakfast reviewed May 2, 2014
View reviewed changes

rwjblue mentioned this pull request May 3, 2014

--port xxxx in .ember-cli is ignored #580

Closed

stefanpenner reviewed May 24, 2014
View reviewed changes

stefanpenner reviewed May 27, 2014
View reviewed changes

stefanpenner reviewed Jun 1, 2014
View reviewed changes

stefanpenner reviewed Jun 7, 2014
View reviewed changes

twokul and others added 7 commits June 11, 2014 09:44

adds yam as a dependency

f6c75c9

adds code & tests

70f1cec

adds .ember-cli file with default options blueprint

65929d1

tweaks and fixes

ffcc692

more tests

6c7d9f5

Swap order when parsing command arugments (so that command-line args …

c8447f2

…take precedence over config

Add MockConfig and add test in command-test to test for getting confi…

8187485

…g options

rwjblue reviewed Jun 11, 2014
View reviewed changes

Updates tests and removes mock config

d277763

rwjblue reviewed Jun 12, 2014
View reviewed changes

rwjblue added a commit that referenced this pull request Jun 12, 2014

Merge pull request #563 from twokul/config

c05c3fb

Adds yam & config file

rwjblue merged commit c05c3fb into ember-cli:master Jun 12, 2014

twokul deleted the config branch June 12, 2014 12:30

Adds yam & config file #563

Adds yam & config file #563

Conversation

twokul commented May 2, 2014

stefanpenner commented May 2, 2014

stefanpenner commented May 2, 2014

twokul commented May 2, 2014

rwjblue commented May 2, 2014

twokul commented May 2, 2014

twokul commented May 2, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fsmanuel commented May 2, 2014

twokul commented May 2, 2014

fsmanuel commented May 2, 2014

stefanpenner commented May 6, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanpenner commented May 24, 2014

Choose a reason for hiding this comment

stefanpenner commented May 27, 2014

rwjblue commented Jun 1, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanpenner commented Jun 5, 2014

rwjblue commented Jun 6, 2014

twokul commented Jun 6, 2014

andycrum commented Jun 7, 2014

Choose a reason for hiding this comment

andycrum commented Jun 8, 2014

rwjblue commented Jun 8, 2014

andycrum commented Jun 8, 2014

andycrum commented Jun 8, 2014

rwjblue commented Jun 11, 2014

andycrum commented Jun 11, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanpenner commented Jun 12, 2014

rwjblue commented Jun 12, 2014

alvincrespo commented Jun 13, 2014