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

eslint: Enable "semi" rule #9736

Merged
merged 1 commit into from
Jul 26, 2018
Merged

Conversation

atiratree
Copy link
Contributor

  • fixes semicolons in all files

after the discussion with @mareklibra in #9466, we concluded it could be useful to have eslint rule for that

@@ -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 }); })}>
Copy link
Member

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(...).

Copy link
Member

@martinpitt martinpitt left a 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.

@@ -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); }}>
Copy link
Member

Choose a reason for hiding this comment

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

Another such case

@@ -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 });
});
Copy link
Member

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() {() =>

@atiratree atiratree force-pushed the eslint-semi branch 2 times, most recently from b7b67ce to a3b93cc Compare July 26, 2018 09:16
@atiratree
Copy link
Contributor Author

This makes sense, thank you! I'd just like to avoid some excessive punctuation and braces for single-line arrow functions.

I guess that is a bit too much. Added "omitLastInOneLineBlock": true option.

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Nice option :-) Thanks!

Copy link
Member

@martinpitt martinpitt left a 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
@atiratree
Copy link
Contributor Author

You are right, sorry I forgot to test it. Added corresponding jshint rule.

@martinpitt martinpitt merged commit 53ea9be into cockpit-project:master Jul 26, 2018
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.

4 participants