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

Added lz4io #33

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Added lz4io #33

wants to merge 3 commits into from

Conversation

darkdragn
Copy link

Added bindings for lz4io as decompressFileDefault and compressFileAdv and compressFileDefault. Updated setup.py to include new headers. Updated test.py with a file test, running md5 against source, and a result of compressing/decompressing.

@darkdragn darkdragn changed the title Added lz4io, bump to r121 Added lz4io, bump to r122 Aug 28, 2014
@steeve
Copy link
Owner

steeve commented Sep 2, 2014

Can you undo the changes to lz4 files? I'd rather have these changes upstream, and simply mirror them here

cc @Cyan4973 for upstream

@steeve
Copy link
Owner

steeve commented Sep 2, 2014

Otherwise, good job!

@darkdragn
Copy link
Author

Thank you, for the compliment!
I know that is going to make me sound ignorant, however I'm new to using git so the terminology is throwing me through a loop. I think all of those changes are from bumping from r119 to r122, and I thought you might want to handle that bump your self, so I have a commit in right before the bump. Did you just want me to roll back to that commit with the lz4 files still in the r119 state? I hope you don't mind, but I'm using this as a learning experience. Thank you very much for letting me contribute!

@darkdragn
Copy link
Author

Steeve, I created a new branch from the earlier r119 commit, which has almost of the lz4 files matching. The only one that doesn't is lz4hc.h, since your version is r91 not r119. lz4io requires some functions that are only present in r119 header, so adding the r119 lz4hc.h was a necessity, sorry. If you want, I can close this pull request and do a new request from the branch matching all of your r119 files.

@darkdragn darkdragn changed the title Added lz4io, bump to r122 Added lz4io Sep 4, 2014
@darkdragn
Copy link
Author

Just wanted to leave you with a side note. On the compressFileAdv, for the docstring I did not list blockMode because with r119 it segfaults on chained blocks. On r122 it does not. Once this is commited, and updated to r122, I'll add blockMode to the docstring. It's still in the ParseTupleAndKeywords as an option, I just don't want them using it until the update to r122.

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.

2 participants