search

pistacheio / pistache

A high-performance REST toolkit written in C++
https://pistacheio.github.io/pistache/
Apache License 2.0
3.17k stars 698 forks source link

fix: make zlib a private dependency #1176

Closed Tachi107 closed 1 week ago

Tachi107 commented 10 months ago

Before this patch, the zlib.h header was included by pistache/http.h, meaning that users had to install the development headers on zlib to build projects which depend on Pistache (in other terms, it was a transitive, public, API dependency). Since zlib.h was included by http.h just to get the value of zlib's default compression level, I simply hardcoded it to 6; it's unlikely to ever change.

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.09%. Comparing base (31bc54d) to head (b381c22). Report is 217 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1176 +/- ## ======================================= Coverage 78.08% 78.09% ======================================= Files 53 53 Lines 7077 7075 -2 ======================================= - Hits 5526 5525 -1 + Misses 1551 1550 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kiplingw commented 10 months ago

But is this a good idea? Is there any particular reason why the include needs to be removed? Was it having a noticeable impact on preprocessor burden at compile time? You are right that the constant is unlikely to change, but it still could. zlib has gone through many changes and likely still will.

kiplingw commented 2 weeks ago

Hey @Tachi107, do you want me to close this or do you need more time?

Tachi107 commented 1 week ago

Adding a normalized 1..0 value independent of the underlying compression algorithm used seems like a better solution (which still needs to be implemented). Closing.