- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15
 
2017 Q2 Proposed JavaScript standards updates #2
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
Conversation
        
          
                linters/.eslintrc
              
                Outdated
          
        
      | ############################################################################################### | ||
| 
               | 
          ||
| "no-param-reassign": [2, {"props": true}], # Disallow reassignment of parameters | ||
| "no-param-reassign": [2, { # Disallow reassignment of parameters | 
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.
I think this line (and a few others in this document) is mixing tabs and spaces for indenting the comments.
| # CLASSES <http://github.com/thenerdery/javascript-standards#classes> | ||
| ############################################################################################### | ||
| 
               | 
          ||
| "no-dupe-class-members": 2, # Disallow duplicate class member names | 
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.
Propose that we remove all inline and section comments . You can easily read about the rules from the eslint site itself. In some editors when there's an eslint warning it will even provide a direct link to the rule.
These comments can also be tricky to maintain when lines are changed.
| 
               | 
          ||
| "no-dupe-class-members": 2, # Disallow duplicate class member names | ||
| "no-class-assign": 2, # Disallow reassigning classes | ||
| "no-useless-constructor": 2, # Disallow unnecessary constructor | 
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.
I liked this rule. If my constructor isn't doing anything why keep it around?
ES2015 provides a default class constructor if one is not specified. As such, it is unnecessary to provide an empty constructor or one that simply delegates into its parent class
| 
           Consider turning this into a sharable config http://eslint.org/docs/developer-guide/shareable-configs.html  | 
    
| 
           Just adding some notes here. A sharable config would allow us to maintain inheritance of airbnb's linting rules but still allowing us to override them with our own rules. Assuming our sharable config is published in the npm ecosystem we could do something like: .eslintrc {
  "extends": [
    "airbnb",
    "./node_modules/@nerdery/linters/eslint.js"
  ]
}Perhaps something a bit fancier: .eslintrc {
  "plugins": [
    "jsx-a11y",
    "react"
  ],
  "extends": [
    "airbnb",
    "plugin:react/recommended",
    "plugin:jsx-a11y/recommended",
    "./node_modules/@nerdery/linters/eslint.js"
  ]
} | 
    
No description provided.