New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable loading configuration from `.babelrc`. #145

Merged
merged 2 commits into from Jun 1, 2017

Conversation

Projects
None yet
4 participants
@aheuermann
Copy link
Contributor

aheuermann commented May 16, 2017

Fixes #92

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented May 16, 2017

Ok, just landed a PR that should have fixed CI, can you rebase this so we can get things green before landing?

@aheuermann aheuermann force-pushed the aheuermann:disable-babelrc branch from b049e9c to 2c1162c May 16, 2017

@aheuermann

This comment has been minimized.

Copy link
Contributor Author

aheuermann commented May 16, 2017

Updated. Appveyor tests are still failing for node 4. I assume it is unrelated.

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented May 16, 2017

Yes, the failure seems to be during the npm install phase, and therefore pretty unlikely.

@rwjblue rwjblue requested review from stefanpenner and Turbo87 May 16, 2017

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented May 16, 2017

This change will disable ember-cli-babel from attempting to load configuration from .babelrc and package.json. That functionality was never intended to be enabled (I didn't realize this flag had to be specific disabled), but has worked for the duration of ember-cli-babel@6 so far. A number of folks have opened issues related to this and it is tricky to even know this is going on, but technically it could be considered a breaking change. I personally would categorize it as a "breaking bugfix", and think we should land it.

@stefanpenner / @Turbo87 - I'm 👍 on this, but want y'all to be aware and confirm that you agree with this change also before landing.

@Turbo87
Copy link
Contributor

Turbo87 left a comment

seems good. should we explicitly mention this in the readme?

@aheuermann

This comment has been minimized.

Copy link
Contributor Author

aheuermann commented May 17, 2017

@Turbo87 @rwjblue I added a line to the readme.

README.md Outdated
@@ -20,7 +20,8 @@ target browsers (as specified in `config/targets.js` in ember-cli >= 2.13). Runn
through the transpiler shouldn't change the code at all (likely just a format change if it does).

If you need to customize the way that `babel-preset-env` configures the plugins that transform your code,
you can do it by passing in any of the options found [here](https://github.com/babel/babel-preset-env#options).
you can do it by passing in any of the options found [here](https://github.com/babel/babel-preset-env#options).
*Note: `.bablerc` files are ignored by default.*

This comment has been minimized.

@rwjblue

rwjblue May 17, 2017

Member

Small typo here s/bablerc/babelrc/.

This comment has been minimized.

@aheuermann

aheuermann May 17, 2017

Author Contributor

Fixed

@aheuermann aheuermann force-pushed the aheuermann:disable-babelrc branch from 0adb1f4 to e518a51 May 17, 2017

@rwjblue rwjblue force-pushed the aheuermann:disable-babelrc branch from e518a51 to c701403 Jun 1, 2017

@rwjblue

This comment has been minimized.

Copy link
Member

rwjblue commented Jun 1, 2017

Rebased to kick off another round of CI.

@rwjblue

rwjblue approved these changes Jun 1, 2017

@stefanpenner
Copy link
Member

stefanpenner left a comment

:shipit:

@rwjblue rwjblue merged commit 9ad43a0 into babel:master Jun 1, 2017

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Turbo87 Turbo87 added the internal label Oct 27, 2017

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