-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
eslint: Enable "semi" rule #9736
Conversation
pkg/apps/application-list.jsx
Outdated
@@ -69,7 +69,7 @@ class ApplicationRow extends React.Component { | |||
{comp.summary} | |||
<div className="alert alert-danger alert-dismissable"> | |||
<button className="close" | |||
onClick={left_click(() => { this.setState({ error: null }) })}> | |||
onClick={left_click(() => { this.setState({ error: null }); })}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO this is a bit over the top. I think this should rather drop the unnecessary { } and the semicolon for a single-line function call. I. e. () => this.setState(...)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense, thank you! I'd just like to avoid some excessive punctuation and braces for single-line arrow functions.
pkg/storaged/storage-controls.jsx
Outdated
@@ -160,7 +160,7 @@ var StorageBlockNavLink = React.createClass({ | |||
var parts = utils.get_block_link_parts(client, block.path); | |||
|
|||
var link = ( | |||
<a role="link" tabIndex="0" onClick={() => { cockpit.location.go(parts.location) }}> | |||
<a role="link" tabIndex="0" onClick={() => { cockpit.location.go(parts.location); }}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another such case
pkg/storaged/storage-controls.jsx
Outdated
@@ -185,7 +185,7 @@ class StorageOnOff extends React.Component { | |||
var promise = self.props.onChange(val); | |||
if (promise) { | |||
promise.always(function() { | |||
self.setState({ promise: null }) | |||
self.setState({ promise: null }); | |||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and another one; also, function() {
→ () =>
b7b67ce
to
a3b93cc
Compare
I guess that is a bit too much. Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice option :-) Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no, package fails to build now on "missing semicolon" rules.
- fix semicolons in all files
You are right, sorry I forgot to test it. Added corresponding jshint rule. |
after the discussion with @mareklibra in #9466, we concluded it could be useful to have eslint rule for that