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.

Sign up for GitHub

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

Jump to bottom

Remove LegacyMsgPack codec #218

Merged
merged 5 commits into from Apr 1, 2020
Merged

Remove LegacyMsgPack codec #218

merged 5 commits into from Apr 1, 2020

Conversation

jrbourbeau
Copy link
Member

Closes #217

TODO:

  • Unit tests and/or doctests in docstrings
  • tox -e py38 passes locally
  • tox -e py27 passes locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • tox -e docs passes locally
  • AppVeyor and Travis CI passes
  • Test coverage to 100% (Coveralls passes)

@jakirkham
Copy link
Member

Thanks James! Looks good. 😀

Do we know what’s up with the coverage?

@jrbourbeau jrbourbeau changed the title Remove LegacyMsgPack codec [WIP] Remove LegacyMsgPack codec Mar 27, 2020
@jrbourbeau
Copy link
Member Author

The coverage decrease looks to be coming from the Python 2.7 CI build not passing and contributing it's portion to coverage (e.g. https://coveralls.io/builds/29639832/source?filename=numcodecs%2Fcompat.py#L15).

I'll submit a PR removing Python 2 support and that should fix things : )

@jakirkham
Copy link
Member

Ah ok. Yeah that makes sense. Let's get that in first so we can get CI into a working state 🙂

@jakirkham
Copy link
Member

Could you please resolve the conflicts here James? 🙂

@jrbourbeau jrbourbeau changed the title [WIP] Remove LegacyMsgPack codec Remove LegacyMsgPack codec Apr 1, 2020
@jrbourbeau
Copy link
Member Author

Merge conflicts resolved. I also went ahead and removed the test fixture files associated with LegacyMsgPack

@jakirkham
Copy link
Member

Thanks James! 😄

@jakirkham jakirkham merged commit 45d7c8a into zarr-developers:master Apr 1, 2020
@jrbourbeau jrbourbeau deleted the remove-legacy-msgpack branch April 1, 2020 22:56
@alimanfoo alimanfoo mentioned this pull request Apr 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dropping LegacyMsgPack
2 participants
@jrbourbeau @jakirkham