-
Notifications
You must be signed in to change notification settings - Fork 712
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
Upgraded eslint & eslint-config-airbnb #2058
Conversation
Here is the {
"extends": "airbnb",
"parser": "babel-eslint",
"env": {
"browser": true,
"jest": true,
"node": true
},
"rules": {
"one-var" : 0,
"one-var-declaration-per-line": 0,
"func-names": 0,
"class-methods-use-this": 0,
"import/prefer-default-export": 0,
"import/no-named-as-default" : 0,
"import/no-webpack-loader-syntax": 0,
"import/no-extraneous-dependencies": [
"error",
{
"devDependencies": true,
"optionalDependencies": true,
"peerDependencies": true
}
],
"react/jsx-filename-extension": [
2,
{
"extensions": [
".js",
".jsx"
]
}
],
"comma-dangle": 0,
"no-param-reassign": 0,
"object-curly-spacing": 0,
"react/jsx-closing-bracket-location": 0,
"react/prefer-stateless-function": 0,
"react/sort-comp": 0,
"react/prop-types": 0,
"jsx-a11y/no-static-element-interactions" : 0
}
} |
eslint
& eslint-config-airbnb
eslint
& eslint-config-airbnb
DETAILS_PANEL_WIDTH as WIDTH, | ||
DETAILS_PANEL_OFFSET as OFFSET, | ||
DETAILS_PANEL_MARGINS as MARGINS | ||
} from '../constants/styles'; |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
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.
Mostly just questions for you @fbarl . Nice work!
@@ -13,6 +13,22 @@ const navbarHeight = 194; | |||
const marginTop = 0; | |||
|
|||
|
|||
function renderEmptyTopologyError(show) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
value={value} onChange={this.handleChange} | ||
onFocus={this.handleFocus} onBlur={this.handleBlur} | ||
disabled={disabled} ref="queryInput" /> | ||
disabled={disabled} ref={this.saveQueryInputRef} /> |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
||
require('font-awesome-webpack'); | ||
require('../styles/main.less'); | ||
require('../images/favicon.ico'); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
"eslint-config-airbnb": "13.0.0", | ||
"eslint-loader": "1.6.1", | ||
"eslint-plugin-import": "2.2.0", | ||
"eslint-plugin-jasmine": "2.2.0", |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@@ -4,6 +4,21 @@ import classNames from 'classnames'; | |||
|
|||
import { toggleGridMode } from '../actions/app-actions'; | |||
|
|||
|
|||
function renderItem(icons, label, isSelected, onClick) { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
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.
Massive changeset, and a good step forward. A quick functional test of the UI did not reveal any errors.
I think renaming those JSX functions would add additional clarity.
Otherwise LGTM
LGTM |
…use-this' lint rule
eba7013
to
27ace10
Compare
There is a bunch of new lint rules that I temporarily switched off, because they either require changes across multiple files or I wasn't sure whether we wanted to override them in some way.
@davkal, @foot: I will be working on resolving some of the items from the list on my own now, but I'm sure there will be at least a couple of them which we'll need to discuss. As I expect we'll soon be upgrading
eslint
onservice-ui
as well, maybe @jpellizzari & @bowenli would also like to contribute if they feel we should disable/override some of these rules:import/firstusedimport/prefer-default-export- disabledimport/no-extraneous-dependencies- used, but only fordependencies
, which is forcing the used D3 modules to be listed explicitly inpackage.json
; the default rule would force us to move somereact
dev stuff fromdevDependencies
todependencies
import/no-named-as-defaultusedjsx-a11y/no-static-element-interactions- disabled, at least for now as it applies to a lot of elements and in some cases it seems too strictjsx-a11y/label-has-for- usedreact/jsx-indentusedreact/jsx-filename-extension- tweaked like @jpellizzari posted belowreact/jsx-first-prop-new-lineusedreact/jsx-no-target-blank- usedreact/no-find-dom-node- usedreact/no-string-refsusedreact/self-closing-compusedreact/forbid-prop-types- usedno-plusplususedno-mixed-operators- usedno-restricted-properties- disabled, as we get warnings for using**
instead ofMath.pow
which is only a part of ES2017 syntax that Airbnb uses, but we don'tno-underscore-dangle- usedarrow-parens- usedclass-methods-use-this- used, but pulled such functions out of component logic and made them helpers instead of class static methodsglobal-require- disabled because of how our hot loading is set upNote: I will be removing the items from this list as I resolve them