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

remove de_json() #789

Merged
merged 3 commits into from Sep 1, 2017

Conversation

Projects
None yet
2 participants
@Eldinnie
Member

Eldinnie commented Aug 11, 2017

Remove unneeded and unused de_json and de_list methods from our library. The only usages for these methods in our lib were within each other's de_json methods and our tests.
Expect Travis to fail, let it run again when #788 is merged.

Fixes #778

@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Aug 12, 2017

Member

tests passed. review wanted

Member

Eldinnie commented Aug 12, 2017

tests passed. review wanted

@jsmnbom

Looks good to me, I think you got all the classes. Left two very minor comments though.

Show outdated Hide outdated tests/test_inputtextmessagecontent.py
Show outdated Hide outdated tests/test_inputlocationmessagecontent.py
@Eldinnie

This comment has been minimized.

Show comment
Hide comment
@Eldinnie

Eldinnie Aug 12, 2017

Member

@bomjacob I addressed your review changes.

Member

Eldinnie commented Aug 12, 2017

@bomjacob I addressed your review changes.

@jsmnbom jsmnbom added this to the 8.0 milestone Aug 26, 2017

@Eldinnie Eldinnie merged commit eae139d into master Sep 1, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+3.5%) to 94.267%
Details

@Eldinnie Eldinnie deleted the remove_de_json branch Sep 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment