search

Sygmei / 11Zip

Dead simple zipping / unzipping C++ Lib
MIT License
92 stars 20 forks source link

Add UnitTests #10

Closed Assertores closed 2 years ago

Assertores commented 2 years ago

also fixes #9

and also adds a .clang-format file

Sygmei commented 2 years ago

Hey @Assertores ! Thanks a lot for this PR. I don't see what exactly fixed your issue, was it the unnecessary defines here ? Also I don't think that embedding a zip containing an executable directly in the repository is a good idea, maybe that test CMake could build a dummy executable and zip it ? If you don't want to do it, I can handle this part. Also the CI doesn't seem to run the tests, only build them ?

Assertores commented 2 years ago

yes removing the unnecessary set() solves the issue. the reason i added them in the first place was because ninja has issues building it without the set() but msvc and gcc (on linux) seam to work. therefore i removed them again.

i'll remove the zip and create it on the fly as you sayed.

Assertores commented 2 years ago

hi, i added a .clang-format file and reformatted your code as well. i tried to stay as close to your original format as possible. would you mind checking if the code style is ok for you?

Sygmei commented 2 years ago

Looks really good to me, thanks a lot for this high-quality PR :)