Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Intl.Pluralrules #262

Merged
merged 18 commits into from
Apr 6, 2017
Merged

Intl.Pluralrules #262

merged 18 commits into from
Apr 6, 2017

Conversation

zbraniecki
Copy link
Contributor

Ok, let's start the PR.

@zbraniecki
Copy link
Contributor Author

It seems to cover the whole spec, but I'm not sure about how the make-plurals are supposed to work, as when I try to just launch it, it errors with:

/home/zbraniecki/projects/intl/Intl.js/node_modules/make-plural/es6/plurals.js:22
export default {
^^^^^^
SyntaxError: Unexpected token export
    at Object.exports.runInThisContext (vm.js:78:16)
    at Module._compile (module.js:543:28)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/home/zbraniecki/projects/intl/Intl.js/lib/core.js:5:31)
    at Module._compile (module.js:571:32)

@eemeli recommendation ( zbraniecki#1 (comment) ) is:

babel-node --presets babel-preset-es2015 --ignore regenarator-transform index.js

which I'm not sure if it'll fly with the whole Intl.js requiring it to run.

@zbraniecki
Copy link
Contributor Author

@caridy - but except of this error, I think the PR is ready for review.

@caridy
Copy link
Collaborator

caridy commented Jan 16, 2017

Any update on this @zbraniecki ?

@zbraniecki
Copy link
Contributor Author

So, the PR is ready, but there's a bug on travis that I don't understand, and when I'm trying to manually test using node ./index.js I get a different error, and @eemeli is suggesting a spell to solve it that doesn't work.

It seems like both errors are related to babel and I'm not familiar with babel at all, so I don't know how to fix either of them.

Need help.

@eemeli
Copy link

eemeli commented Jan 16, 2017

Hopefully fixed with zbraniecki#2.

Fix make-plural import path, linter complaints & typos
@zbraniecki
Copy link
Contributor Author

The travis is fixed with zbraniecki#2 thank you @eemeli !

So it seems like this PR should be ready to be merged

@zbraniecki
Copy link
Contributor Author

I applied availableLocales from make-plurals. I have no idea why travis failed now...

@zbraniecki
Copy link
Contributor Author

@caridy ?

@shouze
Copy link

shouze commented Feb 23, 2017

Would appreciate to get this merged, how could I help?

@zbraniecki
Copy link
Contributor Author

It's ready to land as far as I can tell. Pending @caridy to calm travis down and land it.

@shouze
Copy link

shouze commented Mar 15, 2017

ping @caridy @andyearnshaw what can we do to see this issue merged soon? Should @zbraniecki be granted to merge PR on this repo?

@MikaFima
Copy link

MikaFima commented Apr 5, 2017

Hi, this feature is a real need by there. Can we hope a merge soon ?

intlObj['[[maximumFractionDigits]]'] = mxfd;

// 12. If mnsd is not undefined or mxsd is not undefined, then
if (mnsd !== undefined || mxsd !== undefined) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@zbraniecki we changed this one a little bit I think, can you update the implementation?

@caridy caridy merged commit a0c3b3e into andyearnshaw:master Apr 6, 2017
@shouze
Copy link

shouze commented Apr 6, 2017

Thank you so much @caridy! Do you plan to make a release too?

Would be able to upgrade https://github.com/Financial-Times/polyfill-service/issues/1097 this way.

@caridy
Copy link
Collaborator

caridy commented Apr 6, 2017

@shouze unfortunately I can't do it this week, but if there is anyone willing to help with the release, I can help :)

@shouze
Copy link

shouze commented Apr 11, 2017

ok so @caridy I can help what needs to be done to publish a 1.2.6 except publishing to npm repo?

It's been quite a long time that 1.2.6 has been prepared (af235a6) in fact so it should be released ASAP no?

Please tell me in any way I can help, can be your hands to make things happen! 👼

@caridy
Copy link
Collaborator

caridy commented Apr 11, 2017

@aurambaj
Copy link

Hi, any plan to release this soon?

@piuccio
Copy link

piuccio commented Oct 18, 2018

Am I correct in saying that this PR is not published yet on npm?

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.

7 participants