Skip to content
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

fixes #7 #12

Closed
wants to merge 2 commits into from
Closed

fixes #7 #12

wants to merge 2 commits into from

Conversation

aruberto
Copy link
Contributor

No description provided.

@aruberto aruberto mentioned this pull request Jan 19, 2016
@rxaviers
Copy link
Owner

@alunny, this change is supposed to fix the plugin for Windows env. I want to make sure it doesn't break it for you. Could you please check? Thanks

@flipchart
Copy link

@rxaviers I'm not the OP, but can confirm this works for me on WIndows (and is required in order to get webpack building with Globalize).

@rxaviers
Copy link
Owner

Thank you @flipchart, I want to make sure it doesn't break other envs (which I guess it won't). I plan to merge this PR soon. Just finding time to do this final review.

@alunny
Copy link
Collaborator

alunny commented Jan 29, 2016

Thanks @rxaviers, will confirm today.

@@ -5,7 +5,7 @@ var path = require("path");
var mainFiles = ["ca-gregorian", "currencies", "dateFields", "numbers", "timeZoneNames", "units"];

var isGlobalizeModule = function(filepath) {
filepath = filepath.split( "/" );
filepath = filepath.split( /[\/\\]/ );
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use path.sep in node to get the platform specific file separator (a bit of a pain with some of the regex literals in this diff though).

@alunny
Copy link
Collaborator

alunny commented Jan 29, 2016

Diff works with our app, thanks for the PR @aruberto :)

@texttechne
Copy link

I can also confirm that the fix works.

@rxaviers Any chance to get this merged and released in the near future? IMHO it is more or less a critical bug...

@rxaviers rxaviers closed this in de47d5a Mar 8, 2016
@rxaviers
Copy link
Owner

rxaviers commented Mar 8, 2016

Thank you @aruberto and everyone that checked this fix.

@rxaviers
Copy link
Owner

rxaviers commented Mar 8, 2016

Released as 0.3.2. Sorry for the delay.

@texttechne
Copy link

wow, that was amazingly fast! Thanks!!!

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.

5 participants