Skip to content

Fix some warnings found by quantifiedcode#8027

Merged
andrey-git merged 5 commits into
home-assistant:devfrom
andrey-git:cleanup
Jun 16, 2017
Merged

Fix some warnings found by quantifiedcode#8027
andrey-git merged 5 commits into
home-assistant:devfrom
andrey-git:cleanup

Conversation

@andrey-git
Copy link
Copy Markdown
Contributor

@andrey-git andrey-git commented Jun 14, 2017

Description:

  • Fix some warnings found by quantifiedcode
  • Delete instean_hub component which has been disabled for a while.

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

Comment thread homeassistant/components/insteon_hub.py Outdated
# discovery.load_platform(hass, 'light', DOMAIN, {}, config)

return True
# return True
Copy link
Copy Markdown
Member

@pvizeli pvizeli Jun 14, 2017

Choose a reason for hiding this comment

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

You can not comment out this code on insteon_hub

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Al lthis code is unreachable due to return False above.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see a benefit for do that and also why we should produce a precidence case with it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Delete the hole component are better as comment out that stuff.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer if we just don't touch this file. If anything, just add a comment to disable the Quantified Code warning.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Although I guess we can also just remove it all.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Jun 15, 2017

I think the change are legitime until that one with comment a lot of lines out see comments above.

@andrey-git
Copy link
Copy Markdown
Contributor Author

I deleted the instean_hub component.

@andrey-git andrey-git merged commit 1fde234 into home-assistant:dev Jun 16, 2017
@andrey-git andrey-git deleted the cleanup branch June 16, 2017 19:44
@balloob balloob mentioned this pull request Jul 1, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants