Web-console: fix alerts from lgtm #8346
Conversation
|
This pull request introduces 1 alert and fixes 10 when merging f5abf1d into cb1339e - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 2 alerts and fixes 11 when merging 3ee410c into 6fa22f6 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request fixes 10 alerts when merging 85c5d77 into 14a4238 - view on LGTM.com fixed alerts:
|
|
This pull request fixes 23 alerts when merging 3be0d94 into d5a1967 - view on LGTM.com fixed alerts:
|
| @@ -21,6 +21,7 @@ import axios from 'axios'; | |||
| import copy from 'copy-to-clipboard'; | |||
| import React from 'react'; | |||
|
|
|||
| import { Loader } from '..'; | |||
There was a problem hiding this comment.
could you not import from ..
| if (!columnTree) return null; | ||
| const currentSchemaSubtree = | ||
| columnTree[selectedTreeIndex > -1 ? selectedTreeIndex : 0].childNodes; | ||
| console.log(currentSchemaSubtree); |
| @@ -67,9 +67,10 @@ export class StackedBarChart extends React.Component<StackedBarChartProps, Stack | |||
|
|
|||
| componentWillReceiveProps(nextProps: StackedBarChartProps): void { | |||
| if (nextProps !== this.props) { | |||
There was a problem hiding this comment.
pretty sure this if statement does nothing
web-console/webpack.config.js
Outdated
| @@ -19,7 +19,6 @@ | |||
| const process = require('process'); | |||
| const path = require('path'); | |||
| const postcssPresetEnv = require('postcss-preset-env'); | |||
| const BundleAnalyzerPlugin = require('webpack-bundle-analyzer').BundleAnalyzerPlugin; | |||
There was a problem hiding this comment.
rather than totally removing this please tie it to an env variable
|
LGTM after CI 👍 |
| plugins: [ | ||
| // new BundleAnalyzerPlugin() | ||
| ], | ||
| plugins: process.env.BUNDLE_ANALYZER_PLUGIN === 'TRUE' ? [new BundleAnalyzerPlugin()] : [], |
There was a problem hiding this comment.
@vogievetsky is this what you wanted? I documented it in both the build script and the read me
|
This pull request fixes 23 alerts when merging 90f06f6 into d5a1967 - view on LGTM.com fixed alerts:
|
LGTM is reporting a bunch of alerts this should fix a bunch of them.