Skip to content

Feature flags#180

Merged
rwjblue merged 1 commit intoember-cli:masterfrom
201-created:feature-flags
Mar 26, 2017
Merged

Feature flags#180
rwjblue merged 1 commit intoember-cli:masterfrom
201-created:feature-flags

Conversation

@mixonic
Copy link
Member

@mixonic mixonic commented Mar 25, 2017

Add infrastructure for feature flags. This change should have no impact on applications currently consuming Ember-resolver. A PR to follow will add an initial implementation of the module unification feature behind this tooling.

README.md Outdated
## Installation

Ember-resolver was previously a bower package, but since v1.0.1, it has become an ember-cli addon, and should be installed with `ember install`:
Ember-resolver is an ember-cli addon, and should be installed with `ember install`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit picky, but seems odd to hyphenate and capitalize.

flag settings change how `ember-resolver` compiles into an ember-cli's
`vendor.js`, so you should think of them as an application build-time option.

Feature flags are set in an application's `config/environment.js`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we change this to use ember-cli-build.js instead (no need to leak these to the browser), but we should do that in a follow up I think...

README.md Outdated

module.exports = function(environment) {
var ENV = {
emberResolver: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it is annoying, but I generally prefer to use the exact package name for the key here:

ENV = {
  'ember-resolver': { }
}

/* ... */
```

Note that you must restart your ember-cli server for changes to the `flags` to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we move to ember-cli-build.js this becomes more obvious, since no changes to ember-cli-build.js are detected and updated without a restart today.

index.js Outdated
},
envFlags: {
source: 'ember-resolver-env-flags',
flags: { DEBUG: options.environment !== 'production' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be process.env.EMBER_ENV != 'production', not sure what options.environment is here?

index.js Outdated
plugins: [
[require('babel-plugin-debug-macros').default, {
debugTools: {
source: 'ember-debug'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use the JS module unification module here I think, which would be @ember/debug.

@mixonic mixonic force-pushed the feature-flags branch 2 times, most recently from 0fb0cb2 to 63a025f Compare March 25, 2017 17:47
@mixonic
Copy link
Member Author

mixonic commented Mar 25, 2017

@rwjblue I've clean up a few things per your notes

@rwjblue
Copy link
Member

rwjblue commented Mar 25, 2017

Travis is failing for a parse error, can you double check?

@mixonic mixonic force-pushed the feature-flags branch 4 times, most recently from cd24ad2 to 24dbb76 Compare March 25, 2017 18:32
@mixonic
Copy link
Member Author

mixonic commented Mar 25, 2017

Blocked on ember-cli/babel-plugin-debug-macros#30

@rwjblue
Copy link
Member

rwjblue commented Mar 25, 2017

@mixonic - ember-cli/babel-plugin-debug-macros#30 is merged and published, can you update versions here to kick off CI again?

@mixonic
Copy link
Member Author

mixonic commented Mar 26, 2017

@rwjblue this is good to go!

@rwjblue rwjblue merged commit 9efec75 into ember-cli:master Mar 26, 2017
@mixonic mixonic deleted the feature-flags branch March 26, 2017 01:18
@mixonic
Copy link
Member Author

mixonic commented Mar 26, 2017

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants