search

BYVoid / uchardet

An encoding detector library ported from Mozilla
Other
609 stars 107 forks source link

Use GNUInstallDirs cmake module, fix library filename bug, minor cleanups #30

Closed Coacher closed 8 years ago

Coacher commented 8 years ago

Builds, installs and passes tests fine on my machine.

Jehan commented 8 years ago

I have not check everything yet, but why bump the minimum cmake version to 2.8.5? I am never against using newer version of any dependency but there still needs to be reasons why things are done. :-)

Coacher commented 8 years ago

See the next commit: https://github.com/BYVoid/uchardet/pull/30/commits/020eab1ca74fae88a775c4e373b76f2fdf85e352

Coacher commented 8 years ago

I can squash the first and the second commits together, or add a hint to the first one towards the second one.

Jehan commented 8 years ago

You don't need to squash the commits. It's good to keep individual changes. But yes, adding the info of which features requires the version bump in the commit message where the bump appears is a good idea. Thanks!

Coacher commented 8 years ago

OK, I've updated the commit message of the first commit to clarify why the bump is needed.

Jehan commented 8 years ago

Hi. Ok I reviewed this all, and tested. It looks good, except for a the pkg-config template which should be updated with the new CMAKE_ variable (so right now calls to pkg-config are broken for uchardet). I'll fix this. No prob. Other than this, looks like a cool improvement to the build system.

Jehan commented 8 years ago

pkg-config file fixed with commit fb1d544. Thanks for the patches!

Coacher commented 8 years ago

Sorry, I totally forgot about pkg-config template. Thank you.