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

Update ESLint and enable more rules #572

Merged
merged 9 commits into from
Aug 10, 2016
Merged

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Aug 10, 2016


const EmberDebug = EmberObject.extend({

application: null,
started: false,

// Using object shorthand syntax here is somehow having strange side effects.
// eslint-disable-next-line object-shorthand
Port: Port,
Copy link
Contributor

@teddyzeenny teddyzeenny Aug 10, 2016

Choose a reason for hiding this comment

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

What were the side effects? My first guess is because the es6 module is being transpiled before Babel so Port import is being transpiled to Port['default'] before Babel gets the file.

Copy link
Member Author

Choose a reason for hiding this comment

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

huh, that might indeed explain things... I've got another PR waiting that ditches the es6module plugin and uses Babel for that like Ember CLI does too. Once that is merged we can remove the comment and use the shorthand syntax.

@teddyzeenny
Copy link
Contributor

❤️ !!!!

@teddyzeenny
Copy link
Contributor

Looks good to me. Is this ready to merge?

@Turbo87
Copy link
Member Author

Turbo87 commented Aug 10, 2016

@teddyzeenny should be ready to merge from my side. Thanks for the quick review!

@teddyzeenny
Copy link
Contributor

Can you squash the ESlint rules into one commit?

@Turbo87
Copy link
Member Author

Turbo87 commented Aug 10, 2016

@teddyzeenny done

@teddyzeenny teddyzeenny merged commit da5219e into emberjs:master Aug 10, 2016
@Turbo87 Turbo87 deleted the eslint branch August 10, 2016 18:06
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.

2 participants