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

Add ember-cli-babel as a dependency and clean up #12

Merged
merged 22 commits into from
Sep 8, 2017

Conversation

YoranBrondsema
Copy link
Collaborator

@YoranBrondsema YoranBrondsema commented Jun 9, 2017

@YoranBrondsema YoranBrondsema changed the title Add ember-cli-babel as a dependency Add ember-cli-babel as a dependency and clean up Jun 9, 2017
@YoranBrondsema
Copy link
Collaborator Author

Tests pass locally but no on Travis :(. @johnotander I don't think I can clear the cache on Travis CI. Would you mind doing that? I have a feeling it's due to that.

"ember-qunit-notifications": "0.0.4",
"qunit": "~1.15.0",
"Faker": "~2.1.0"
"Faker": "4.1.0"

Choose a reason for hiding this comment

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

Faker is available as an npm package, removing all need to keep using Bower.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right. I will look into making this change this weekend.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alanlalonde I tried importing Faker as an NPM module but unfortunately it's only available as a CommonJS module. I couldn't really find an easy way of importing it, in the same way the Bower component is imported (the NPM module does not export a built file). So I'd like to leave it as it is for now, and we can look into importing it as a NPM module in a future PR. In any case, this PR does quite some much needed modernizing!

Choose a reason for hiding this comment

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

That's fine. If it'll require extra work, it can be the focus of the next modernization PR. Thanks for checking to see if it's doable on your end.

@YoranBrondsema
Copy link
Collaborator Author

@johnotander Any updates?

@Subtletree
Copy link

ember-browserify is an option for importing faker from npm

@Subtletree
Copy link

Actually ember-cli-mirage import faker from npm using ember-cli-node-assets.

https://github.com/samselikoff/ember-cli-mirage/blob/master/index.js

@YoranBrondsema
Copy link
Collaborator Author

@Subtletree Perfect, thanks! I still want to merge this PR first and move away from Bower in a next PR.

@johnotander in case you're not actively using this addon yourself anymore, would you mind giving contributor permissions to others so that we can continue maintaining the addon? I think there are still a few projects depending on it! You could start with me so I can get this modernization PR merged :-)

@johno
Copy link
Owner

johno commented Sep 5, 2017

Hey @YoranBrondsema, thanks for volunteering and sorry for the delay in responding. I've sent you an invite to become a collaborator so if you wanted to take over the maintenance of this project I'd be delighted 💓 . I've unfortunately been short on time for maintaining some of my libraries lately.

@johno
Copy link
Owner

johno commented Sep 5, 2017

Also, if you send along your npm username I can add you as an owner of the package so you can push new versions.

@YoranBrondsema
Copy link
Collaborator Author

@johnotander No problem, thanks! My NPM username is yoranbrondsema

@johno
Copy link
Owner

johno commented Sep 6, 2017

Added you as an owner to the npm package ❤️

@YoranBrondsema YoranBrondsema merged commit c7b518d into johno:master Sep 8, 2017
@bgentry bgentry mentioned this pull request Sep 27, 2017
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.

4 participants