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 the no-get rule to also handle the getProperties function, and mark the no-get-properties rule as deprecated #421

Merged
merged 1 commit into from May 26, 2019

Conversation

bmish
Copy link
Member

@bmish bmish commented May 15, 2019

Based on the discussion in #415 and #419, we decided to make the no-get-properties rule more generic, but then it becomes almost identical to the no-get rule, so then we should just consolidate it into the no-get rule (which can handle both this.get and this.getProperties). Both rules are intended to enforce the new ES5 getter syntax, and there's no need to have two separate rules for enforcing the same exact best practice.

@Turbo87
Copy link
Member

Turbo87 commented May 15, 2019

Note that the no-get-properties rule is new and has only been around for one short release so I think it's best to simply delete it now.

if it's released we can't remove it anymore unfortunately :-/

@bmish bmish changed the title Combine no-get-properties rule into no-get rule Update the no-get rule to handle getProperties and mark no-get-properties as deprecated May 15, 2019
@bmish
Copy link
Member Author

bmish commented May 15, 2019

@Turbo87 good point, I updated the PR to leave no-get-properties in place but mark it as deprecated instead.

@bmish
Copy link
Member Author

bmish commented May 21, 2019

@pavlikmarek would you like to review since you worked on the related PR? I'll get this released once it's approved.

@bmish
Copy link
Member Author

bmish commented May 24, 2019

@Turbo87 will plan to merge this in the next few days unless you have more feedback.

@Turbo87
Copy link
Member

Turbo87 commented May 24, 2019

👍

Copy link

@pavlikmarek pavlikmarek left a comment

Choose a reason for hiding this comment

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

Perfect, good job! 👏

…k the `no-get-properties` rule as deprecated
@bmish bmish merged commit 85bac54 into ember-cli:master May 26, 2019
@bmish bmish changed the title Update the no-get rule to handle getProperties and mark no-get-properties as deprecated Update the no-get rule to also handle the getProperties function, and mark the no-get-properties rule as deprecated May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants