don't use local Babel directly #321
Conversation
| @@ -363,13 +363,15 @@ test(t => { | |||
| }); | |||
| ``` | |||
|
|
|||
| You can also use your own local Babel version: | |||
| To use your own local Babel version, specify the version you want in your package.json, and ensure you're using npm@3 (or run `$ npm dedupe`), so the dependency will be deduped. | |||
There was a problem hiding this comment.
I assume you're advocating deduping so that if the code-under-test requires babel-core it doesn't load two versions? If so, is that really worth mentioning?
There was a problem hiding this comment.
No, I'm advocating dedupe so the user is able to set an explicit Babel 5 version that we will also use. Since we depend on a loose Babel 5 version, the user can set an exact version and it will be deduped to the user's choice.
There was a problem hiding this comment.
Ah, thanks for elaborating. I did not find that obvious from the documentation though. Same with the version pinning below which follows a note on Babel 6 support.
There was a problem hiding this comment.
Good feedback. I'll try to make that clearer. Any suggestions?
There was a problem hiding this comment.
Since we depend on a loose Babel 5 version, the user can set an exact version and it will be deduped to the user's choice.
This just doesn't seem very relevant to me. AVA uses Babel, fine. The version may change, even in patch releases, as long as the test behavior remains the same. Why should a user be concerned with setting an exact version in the project in order to influence the version AVA uses?
What I find more important as a user is knowing that I can install whatever Babel version I like and it doesn't break AVA.
|
@sindresorhus - ultimately your call on switching the example to Babel 6, otherwise LGTM |
Instead depend on npm to dedupe. This is better as we will no longer fail if the user has Babel 6 and we Babel 5, and npm just won't dedupe. Same situation in the future when we're on Babel 6, but the user is still on Babel 5.
sindresorhus
force-pushed
the
babel-simplify
branch
from
b3abfeb
to
d0c370a
Compare
|
Ok, updated. I went with just removing the example as it didn't give much. |
|
LGTM. |
|
@jamestalmage Not sure. @bradrydzewski ? |
|
@sindresorhus @jamestalmage the original LGTM will suffice. I've gotten a request to support |
|
Like -1 to change your vote? |
|
@jamestalmage yes, exactly. Open to other suggestions as well 😄 |
|
My only other thought would be a conditional approval, maybe using the markdown checklist syntax. LGTM if:
Other collaborators can then check those off as the submitter handles the feedback. I think that would be especially helpful on the more heavily debated PRs with wall-of-text posts, where feedback can be lost amongst the noise. |
|
LGTM |
don't use local Babel directly
<!-- Read the [contributing guidelines](https://github.com/avajs/ava/blob/master/contributing.md). We are excited about pull requests, but please try to limit the scope, provide a general description of the changes, and remember, it's up to you to convince us to land it. If this fixes an open issue, link to it in the following way: `Fixes #321`. New features and bug fixes should come with tests. --> A recipe for using AVA with JSPM, per discussion on #131. It requires the import of an external library I wrote for the purpose of encapsulating the loader shim so it can be maintained and updated. Here's a link to that library: [ava-jspm-loader](https://github.com/skorlir/ava-jspm-loader). Closes #941
<!-- Read the [contributing guidelines](https://github.com/avajs/ava/blob/master/contributing.md). We are excited about pull requests, but please try to limit the scope, provide a general description of the changes, and remember, it's up to you to convince us to land it. If this fixes an open issue, link to it in the following way: `Fixes #321`. New features and bug fixes should come with tests. --> A recipe for using AVA with JSPM, per discussion on #131. It requires the import of an external library I wrote for the purpose of encapsulating the loader shim so it can be maintained and updated. Here's a link to that library: [ava-jspm-loader](https://github.com/skorlir/ava-jspm-loader). Closes #941
Read the [contributing guidelines](https://github.com/avajs/ava/blob/master/contributing.md). We are excited about pull requests, but please try to limit the scope, provide a general description of the changes, and remember, it's up to you to convince us to land it. If this fixes an open issue, link to it in the following way: `Fixes avajs#321`. New features and bug fixes should come with tests.
Instead depend on npm to dedupe. This is better as we will no longer fail if the user has Babel 6 and we Babel 5, and npm just won't dedupe. Same situation in the future when we're on Babel 6, but the user is still on Babel 5.