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.

Sign up for GitHub

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

Jump to bottom

[RFC][NI] Update ember to 2.16 and migrate to the new module API #203

Merged
merged 15 commits into from
Nov 9, 2017
Merged

[RFC][NI] Update ember to 2.16 and migrate to the new module API #203

merged 15 commits into from
Nov 9, 2017

Conversation

AndreJoaquim
Copy link
Contributor

NI

@AndreJoaquim AndreJoaquim force-pushed the NI/update-ember-and-use-new-module-import branch from 9649019 to bc6bdaf Compare October 26, 2017 10:33
@AndreJoaquim AndreJoaquim changed the title [WIP][NI] Update ember to 2.16 and migrate to the new module API [RFC][NI] Update ember to 2.16 and migrate to the new module API Oct 26, 2017
asantos00
asantos00 suggested changes Oct 27, 2017
View reviewed changes
Copy link
Contributor

@asantos00 asantos00 left a comment

Choose a reason for hiding this comment

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

Good job, and good job using ember-try 👏

.gitignore
@@ -14,6 +14,10 @@
/coverage/*
/libpeerconnection.log
npm-debug.log*
yarn-error.log
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can do ./*.log, I don't see us wanting to commit log files

addon/components/uni-modal.js
import Ember from 'ember';
import Component from '@ember/component';
import { observer } from '@ember/object';
import $ from 'jquery';
Copy link
Contributor

Choose a reason for hiding this comment

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

oh 😞

telmaantunes
telmaantunes reviewed Nov 7, 2017
View reviewed changes
.gitignore Outdated
testem.log
.DS_Store
Copy link

Choose a reason for hiding this comment

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

why remove these two?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mea culpa

@AndreJoaquim AndreJoaquim force-pushed the NI/update-ember-and-use-new-module-import branch from 6d63890 to 924898a Compare November 9, 2017 16:35
DaniFrancisco
DaniFrancisco approved these changes Nov 9, 2017
View reviewed changes
Copy link
Contributor

@DaniFrancisco DaniFrancisco left a comment

Choose a reason for hiding this comment

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

🥇

@AndreJoaquim AndreJoaquim merged commit 59c52e9 into master Nov 9, 2017
@AndreJoaquim AndreJoaquim deleted the NI/update-ember-and-use-new-module-import branch November 9, 2017 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@telmaantunes telmaantunes telmaantunes left review comments

@mmelo mmelo mmelo approved these changes

@asantos00 asantos00 asantos00 approved these changes

@DaniFrancisco DaniFrancisco DaniFrancisco approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AndreJoaquim @mmelo @asantos00 @telmaantunes @DaniFrancisco