Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

ESLint additional rules #4186

Merged
merged 15 commits into from Jan 18, 2017
Merged

ESLint additional rules #4186

merged 15 commits into from Jan 18, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Jan 17, 2017

Additional eslint rules so we can stop re-formatting code from others due to differences in interpretation. (Auto-fixes done on failed checks with eslint --fix - insubstantial changes, although a large number automatically found & fixed.)

  1. JSX tag spacing, closing tag to align with opening (if multiple lines)
<SomeTag
  attribute={ something }
  className={ styles.item }
/>
  1. No unneeded block spacing
class Something extends Component {
  static propTypes = { // no blank line before
    ...
  }

  render () { // no blank line padding function after
    if (x == y) { 
       doSomething(); // no blank line before
    }

    switch (foo) {
      case 'bar': // no blank line before
        return <div></div>;
    }
  }
}

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Jan 17, 2017
@ngotchac
Copy link
Contributor

@derhuerst I like this convention, but you might want to say how you feel about it before it's carved in stone

@@ -128,11 +131,13 @@ export default class Application extends Component {
<Button
disabled={ registerBusy }
invert={ registerType !== 'file' }
onClick={ this.onClickTypeNormal }>File Link</Button>
onClick={ this.onClickTypeNormal }
>File Link</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

I know the diff has been produced automatically, but this is still not consistent with the other places. Consider adapting it. – Same below.

@@ -39,7 +38,8 @@ class AccountSelectorItem extends Component {

const icon = (<IdentityIcon
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider another newline here.

@@ -25,7 +25,7 @@ class AccountSelectorContainer extends Component {
render () {
return (<AccountSelector
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, consider another newline here.

@@ -155,7 +160,8 @@ export default class QueryAction extends Component {

validationType={ SIMPLE_TOKEN_ADDRESS_TYPE }
onChange={ this.onChange }
onEnter={ this.onQuery } />)
onEnter={ this.onQuery }
/>)
: (<InputText
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, consider another newline here.

@@ -22,11 +22,10 @@ import Actions from './component';
import { registerToken, registerReset, queryToken, queryReset } from './actions';

class TokensContainer extends Component {

render () {
return (<Actions
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, consider another newline here.

@@ -39,7 +39,7 @@ class Container extends Component {
return (<Application
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, consider another newline here.

@@ -20,11 +20,10 @@ import { connect } from 'react-redux';
import InputText from './input-text';

class InputTextContainer extends Component {

render () {
return (<InputText
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, consider another newline here.

@@ -277,7 +276,7 @@ class WriteContract extends Component {
label='Deploy'
Copy link
Contributor

Choose a reason for hiding this comment

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

This indentation feels inconsistent with the rest.

@derhuerst
Copy link
Contributor

@ngotchac This convention makes sense to me. What I've been having trouble with is how/which variables/props to group, not how much spacing etc.

@jacogr
Copy link
Contributor Author

jacogr commented Jan 17, 2017

@derhuerst Thanks for the run-though, will look at the ones that jumped out, may as well help Mrs. Auto a bit for future sanity.

@ngotchac ngotchac added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 18, 2017
@jacogr jacogr merged commit 08f80f2 into master Jan 18, 2017
@jacogr jacogr deleted the jg-eslint-padding branch January 18, 2017 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants