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

Fix alerts from LGTM about python files #8383

Merged
merged 3 commits into from Sep 6, 2019

Conversation

@asdf2014
Copy link
Member

commented Aug 23, 2019

Description

Fix alerts from LGTM about python files


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths.
  • added integration tests.
  • been tested in a test Druid cluster.

@lgtm-com

This comment has been minimized.

Copy link

commented Aug 23, 2019

This pull request fixes 10 alerts when merging 220a766 into d117bfb - view on LGTM.com

fixed alerts:

  • 5 for Unused import
  • 2 for Variable defined multiple times
  • 1 for Comparison using is when operands support __eq__
  • 1 for Unused local variable
  • 1 for Unnecessary 'else' clause in loop
@vogievetsky

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2019

Let's fix that E !

image

@ccaominh
Copy link
Contributor

left a comment

LGTM 👍

@Fokko
Fokko approved these changes Aug 27, 2019
@lgtm-com

This comment has been minimized.

Copy link

commented Aug 30, 2019

This pull request fixes 10 alerts when merging a7ccdc1 into 9849581 - view on LGTM.com

fixed alerts:

  • 5 for Unused import
  • 2 for Variable defined multiple times
  • 1 for Comparison using is when operands support __eq__
  • 1 for Unused local variable
  • 1 for Unnecessary 'else' clause in loop
@lgtm-com

This comment has been minimized.

Copy link

commented Aug 30, 2019

This pull request fixes 10 alerts when merging 9cd869f into 9849581 - view on LGTM.com

fixed alerts:

  • 5 for Unused import
  • 2 for Variable defined multiple times
  • 1 for Comparison using is when operands support __eq__
  • 1 for Unused local variable
  • 1 for Unnecessary 'else' clause in loop
@asdf2014

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

@himanshug himanshug merged commit bba391a into apache:master Sep 6, 2019

3 of 5 checks passed

LGTM analysis: Java No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
Inspections: pull requests (Druid) TeamCity build finished
Details
LGTM analysis: Python 10 fixed alerts
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@asdf2014 asdf2014 deleted the asdf2014:fix_py branch Sep 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.