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

First step to remove globals in dev src #30

Merged
merged 1 commit into from
Nov 1, 2016

Conversation

vire
Copy link
Collaborator

@vire vire commented Oct 29, 2016

No description provided.

@vire vire mentioned this pull request Oct 30, 2016
Copy link
Member

@michalbcz michalbcz left a comment

Choose a reason for hiding this comment

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

nice work !

@@ -1,11 +1,14 @@
// generated on 2016-09-28 using generator-chrome-extension 0.6.1
import gulp from 'gulp';
Copy link
Member

Choose a reason for hiding this comment

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

why did you replace ES6's import with require in this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have to admit it was unintentional. Since I never used gulp before, I did run it in a wrong way a imports didn't worked for me (to be precise, I've tried to rename gulpfile.babel.js to gulpfile.jswhich caused the behavior with having not import working...

Finally before committing I've renamed the file back to original name and forgot to revert the requires ;( I' will create a PR to bring it back since it's the goal to use ES modules...

But in general it's not good that software behaves differently just based on some string in filename like .babel because it isn't obvious for the first look...

https://markgoodyear.com/2015/06/using-es6-with-gulp/

@michalbcz michalbcz merged commit 0e2a9fe into realreality:master Nov 1, 2016
@vire vire deleted the es6-modules-via-rollup branch November 20, 2016 12:29
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.

None yet

2 participants