Skip to content

Conversation

@felixdivo
Copy link
Collaborator

This PR changes util.load_config() to pass through any unused parameters. It is made more clear in the code what Bus.__new__() is doing and what not. A lot has moved to util.load_config() to remove code and responsibility duplication. The documentation has each been changed to reflect what the methods are actually doing, that was not done correct before.

Based an @randycoulman's comment over here
Fixes#280

@felixdivofelixdivo self-assigned this May 16, 2018
@felixdivofelixdivo requested review from christiansandberg and hardbyte and removed request for hardbyteMay 16, 2018 17:16
@felixdivo
Copy link
CollaboratorAuthor

This is done, but we should write some unit tests before merging this PR to later prevent regression.

Note: Making __new__ a @classmethod was a really nasty bug and took me quite a while to figure out. -.- Don't do this in the future! 😛

@felixdivofelixdivo requested a review from hardbyteMay 27, 2018 17:28
Copy link
Owner

@hardbytehardbyte left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just two debugging print statements to remove.


# the channel attribute should be present in **config
print("DEBUGGING: ", args)
print("DEBUGGING: ", config)
Copy link
Owner

Choose a reason for hiding this comment

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

Remove these (or convert to debug level logs)

Copy link
CollaboratorAuthor

Choose a reason for hiding this comment

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

Ah sure, must've forgotten them there.

@felixdivofelixdivo added this to the 2.2 Release milestone May 27, 2018
@felixdivofelixdivo merged commit 6ebe610 into developMay 28, 2018
@felixdivofelixdivo deleted the fix-280 branch May 28, 2018 05:46
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@felixdivo@hardbyte