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

sort-comp: say "should" instead of "must" #372

Merged
merged 2 commits into from
Jan 6, 2016

Conversation

SystemParadox
Copy link
Contributor

warning: componentWillUpdate must be placed after componentDidMount

Sorting component methods is only ever a style preference. "must" is a bit misleading - it implies that there will be errors in the program. Say "should" instead.

Thanks.

@mjomble
Copy link
Contributor

mjomble commented Jan 2, 2016

It seems you'll need to update tests as well to get the Travis CI build to pass.

@SystemParadox
Copy link
Contributor Author

Oops my bad. Should be sorted now. Thanks.

@yannickcr
Copy link
Member

Mmm. Not really sure about this change or, to be more precise, about the reasons for it :)

Imho it depends how the rule is configured, if configured as warning then should is more appropriate, but if configured as error then it is more likely must (since ESLint will exit with code 1 in this case).

After having a look at ESLint rules, it appears that there is no real rules for messages, should and must are used the same way, whether it is for styling rules or not.

(Same for us, due to the different rules authors and no rules about messages formatting, but that something I'd like to improve in future releases)

@yannickcr yannickcr force-pushed the master branch 2 times, most recently from aa6d3a8 to dbf17a9 Compare January 4, 2016 23:41
@SystemParadox
Copy link
Contributor Author

My reasoning is that "must" is absolutely not appropriate for warnings as it is misleading. The reverse is not true - "should" can also be used for errors without causing confusion. "must" is very definite, whereas "should" is a more lenient and flexible term.

Additionally:

  • eslint is primarily and linting and style tool. It is far more likely that rules will be configured as warnings rather than errors, as they are not essential for program correctness
  • even rules configured as errors are not necessarily fatal, depending how eslint is being used. This is the case in my setup, where eslint is run after the normal compilation step.
  • the sort-comp rule in particular makes very little sense as an error and is likely to only ever be used as a warning

I haven't noticed any other instances yet, but at the moment this rule is causing confusion both for me and my coworkers.

Thanks.

@yannickcr
Copy link
Member

My reasoning is that "must" is absolutely not appropriate for warnings as it is misleading. The reverse is not true - "should" can also be used for errors without causing confusion. "must" is very definite, whereas "should" is a more lenient and flexible term.

That's a sufficient reason for me :)

Thanks for the suggestion, do not hesitate to suggest other improvements to the error messages ;)

yannickcr added a commit that referenced this pull request Jan 6, 2016
Replace "must" by "should" in sort-comp error message
@yannickcr yannickcr merged commit 44ad202 into jsx-eslint:master Jan 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants