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

Fixing device.py so that Travis doesn't fail and added test #80

Merged
merged 4 commits into from
Oct 28, 2017

Conversation

ggravlingen
Copy link
Member

In the current prod version of the code, Travis throws an error in device.py (#76). Oddly, the error wasn't there when the affected lines of code was first merged. This is a fix for that error so that Travis can build again. I've also added a test for the set_predefined_color-method.

@@ -227,3 +227,12 @@ def test_color_device_control():
assert light_control.can_set_kelvin
assert light_control.min_kelvin == 1667
assert light_control.max_kelvin == 25000

def test_setters():

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@@ -227,3 +227,12 @@ def test_color_device_control():
assert light_control.can_set_kelvin
assert light_control.min_kelvin == 1667
assert light_control.max_kelvin == 25000

def test_setters():

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@ggravlingen
Copy link
Member Author

@balloob created a new, clean, branch with minimum commits

@ggravlingen ggravlingen merged commit 151c730 into master Oct 28, 2017
@ggravlingen ggravlingen deleted the fixdevice_v2 branch October 28, 2017 16:46
return self.set_hex_color(color, index=index)
except KeyError:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like swallowing errors. Because now as a caller, I won't know if my call succeeded and that means that I can't communicate this to my users. I think that we should never catch but instead raise as much as possible. That way we leave it to developers that use this lib if they want to swallow errors or show them to users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants