Skip to content
This repository was archived by the owner on Jun 8, 2019. It is now read-only.

Babel 7 upgrade#145

Merged
okuryu merged 7 commits into
formatjs:masterfrom
bj00rn:babel-7-upgrade
Sep 12, 2018
Merged

Babel 7 upgrade#145
okuryu merged 7 commits into
formatjs:masterfrom
bj00rn:babel-7-upgrade

Conversation

@bj00rn

@bj00rn bj00rn commented Aug 23, 2018

Copy link
Copy Markdown
Contributor

Upgrade to Babel 7

Resolves: #146 #132 #122

@ykzts

ykzts commented Aug 28, 2018

Copy link
Copy Markdown
Contributor

https://github.com/yahoo/babel-plugin-react-intl/blob/v2.4.0/.travis.yml#L4-L5

Babel v7 is no longer supported under Node.js v5.

@bj00rn

bj00rn commented Aug 29, 2018

Copy link
Copy Markdown
Contributor Author

@ykzts 👍

Comment thread test/index.js
...BASE_OPTIONS,
...options,
}];
}, uuidv1()];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

somewhat dirty fix, need to add random string suffix to allow plugin to run more than once

@tstirrat15

Copy link
Copy Markdown

Two things: one is that Babel 7.0.0 (i.e. non-beta) is out. Is it worth upgrading directly to that?

The other is that I'd love to see this work go through. What's the status? Is there any way that I can help it along?

@bj00rn

bj00rn commented Aug 30, 2018

Copy link
Copy Markdown
Contributor Author

@tstirrat15 moved to 7.0

@bj00rn

bj00rn commented Aug 31, 2018

Copy link
Copy Markdown
Contributor Author

@ericf what are the chances of having this merged?

Comment thread .travis.yml
node_js:
- 4.2
- 5
- 6

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should have testing on v8 and v10 right now.

@bj00rn bj00rn Sep 4, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

seems a little out-of-scope for this, dont you think? maybe a separate PR for this?

Comment thread package.json Outdated
"author": "Eric Ferraiuolo <edf@ericf.me>",
"dependencies": {
"babel-runtime": "^6.2.0",
"@babel/runtime": "7.0.0",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why it's not semver style like ^7.0.0? any concern?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. thanks

didn't notice. i just ran babel-upgrade https://github.com/babel/babel-upgrade

@redonkulus redonkulus mentioned this pull request Sep 10, 2018

@redonkulus redonkulus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm ok with this. We are going to need to release this as a major version bump.

@okuryu once you approve, we can merge and release.

@okuryu okuryu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@okuryu okuryu merged commit def4084 into formatjs:master Sep 12, 2018
@okuryu okuryu mentioned this pull request Sep 12, 2018
@okuryu

okuryu commented Sep 12, 2018

Copy link
Copy Markdown
Member

I just published babel-plugin-react-intl@3.0.0. Please try to test!

Comment thread src/index.js
* See the accompanying LICENSE file for terms.
*/

import * as p from 'path';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue seems to be here where upath is clearly used in library code, but not added to normal dependencies but only devDependencies.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@serwer-WittGruppe want to test it out and open a PR with the fix?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants