Skip to content

Fixes for the API change of construct v2.9.30#220

Merged
rytilahti merged 3 commits intorytilahti:masterfrom
syssi:feature/fix-for-construct-2.9.30
Feb 13, 2018
Merged

Fixes for the API change of construct v2.9.30#220
rytilahti merged 3 commits intorytilahti:masterfrom
syssi:feature/fix-for-construct-2.9.30

Conversation

@syssi
Copy link
Copy Markdown
Collaborator

@syssi syssi commented Feb 13, 2018

Pinning (construct==2.9.30) must be the consequence for the present. As long as construct hasn't a stable api no pinning will be brinkmanship. 😡

Fixes #217.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 68.04% when pulling 635c97e on syssi:feature/fix-for-construct-2.9.30 into bc9fdcf on rytilahti:master.

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 13, 2018

Coverage Status

Coverage remained the same at 68.04% when pulling 90b5aed on syssi:feature/fix-for-construct-2.9.30 into bc9fdcf on rytilahti:master.

@syssi
Copy link
Copy Markdown
Collaborator Author

syssi commented Feb 13, 2018

@rytilahti If you ACK this PR I will prepare a bugfix release as soon as possible.


class ProntoPulseAdapter(Adapter):
def _decode(self, obj, context):
def _decode(self, obj, context, path):
Copy link
Copy Markdown
Contributor

@yawor yawor Feb 13, 2018

Choose a reason for hiding this comment

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

If the code doesn't use the new path arg, wouldn't it be better to add here (and in other methods) *args, **kwargs to just ignore any extra arguments? It would make it backward compatible with earlier Construct versions.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nice approach! I updated the PR.

@rytilahti
Copy link
Copy Markdown
Owner

Ok, let's merge this and get a new release out. I'll start looking into Kaitai to see if we can replace construct for good, it is getting out of control if our simple protocol handling gets broken this often.

@rytilahti rytilahti merged commit a675ef0 into rytilahti:master Feb 13, 2018
@arekbulski
Copy link
Copy Markdown
Contributor

See #222 . I will help you with getting miio uptodate with construct.

@arekbulski
Copy link
Copy Markdown
Contributor

Using *args **kwargs is not recommended, because if those methods change in the future, it might fail in some silent way. You should undo the 2nd commit.

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.

5 participants