search

libnxz / power-gzip

POWER NX zlib compliant library
23 stars 18 forks source link

SetDictionary functions need testing before making them available #13

Closed abalib closed 4 years ago

abalib commented 4 years ago

Signed-off-by: Bulent Abali abali@us.ibm.com

abalib commented 4 years ago

We didn't test these thoroughly. We should disable them until then. I propose to also apply the commit to the master branch as well

rzinsly commented 4 years ago

I don't think it's a good idea remove useful functions and functionalities from the library, instead we should test it better and report bugs we encounter, also having it available we can get users data if they found issues. The codes seems being working ok, have you found any problems?

abalib commented 4 years ago

We need to run regression to make sure deflateSetDictonary output can be correctly decompressed. I am still looking for the source data that failed. The test is found here https://github.com/libnxz/power-gzip/blob/develop/samples/dict-test.sh

abalib commented 4 years ago

Thank you. I am going to setup some test runs to reproduce fails. I assume it's ok to merge PR for now?

rzinsly commented 4 years ago

Thank you. I am going to setup some test runs to reproduce fails. I assume it's ok to merge PR for now?

Yes

abalib commented 2 years ago

I will look for the test case. But regardless I recognized later that my dictionary design and logic might be incorrect. The worst thing we can do is giving users code that sometimes compresses incorrectly, and they cannot decompress their data later. Until we fix this path, we should return an error. Allow me to read the code and I come back with a better explanation and perhaps a design.