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

assert-arg-order: Rename to no-invalid-debug-function-arguments and detect invalid usages of deprecate and warn too #364

Merged
merged 1 commit into from Dec 29, 2018
Merged

assert-arg-order: Rename to no-invalid-debug-function-arguments and detect invalid usages of deprecate and warn too #364

merged 1 commit into from Dec 29, 2018

Conversation

bmish
Copy link
Member

@bmish bmish commented Dec 27, 2018

Renames the assert-arg-order rule to no-invalid-debug-function-arguments. Generalizes it to detect the assert argument order mistake along with two other similar functions (deprecate and warn).

Requested by @Turbo87.

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

I think we should rename the rule as well, perhaps no-invalid-debug-arguments or validate-debug-function-argument-order?

@bmish bmish changed the title feat: assert-arg-order rule: also detect invalid usages of 'deprecate' and 'warn'. feat: rename 'assert-arg-order' rule to 'no-invalid-debug-arguments' to also detect invalid usages of 'deprecate' and 'warn'. Dec 27, 2018
@bmish
Copy link
Member Author

bmish commented Dec 27, 2018

@rwjblue good idea, I renamed the rule to no-invalid-debug-function-arguments.

@bmish bmish changed the title feat: rename 'assert-arg-order' rule to 'no-invalid-debug-arguments' to also detect invalid usages of 'deprecate' and 'warn'. feat: rename 'assert-arg-order' rule to 'no-invalid-debug-function-arguments' to also detect invalid usages of 'deprecate' and 'warn'. Dec 28, 2018

assert(label, 'Label must be present.');
warn(label, 'Label must be present.');
deprecate(title, 'Title is no longer supported.');
Copy link
Member

Choose a reason for hiding this comment

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

@bmish I think deprecate() is supposed to always have a third argument (an options hash).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, added the third argument to both warn and deprecate.

…guments' to also detect invalid usages of 'deprecate' and 'warn'.
@Turbo87 Turbo87 merged commit aaa9cc0 into ember-cli:master Dec 29, 2018
@Turbo87
Copy link
Member

Turbo87 commented Dec 29, 2018

thanks again @bmish!

one small potential improvement would be to not show the generic error message, but instead, show an error message mentioning the debug function that you actually used.

@bmish
Copy link
Member Author

bmish commented Dec 29, 2018

@Turbo87 done: #365

@Turbo87 Turbo87 changed the title feat: rename 'assert-arg-order' rule to 'no-invalid-debug-function-arguments' to also detect invalid usages of 'deprecate' and 'warn'. assert-arg-order: Rename to 'no-invalid-debug-function-arguments' and detect invalid usages of 'deprecate' and 'warn' too Jan 12, 2019
@Turbo87 Turbo87 changed the title assert-arg-order: Rename to 'no-invalid-debug-function-arguments' and detect invalid usages of 'deprecate' and 'warn' too assert-arg-order: Rename to no-invalid-debug-function-arguments and detect invalid usages of deprecate and warn too Jan 12, 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