search

ulfjack / ryu

Converts floating point numbers to decimal strings
Apache License 2.0
1.2k stars 100 forks source link

a simple makefile for the C version #122

Closed gritzko closed 5 years ago

gritzko commented 5 years ago

Bazel is a big overshoot for such a compact project. I link ryu through CMake's ExternalProject, so having a Makefile simplifies things a lot.

ulfjack commented 5 years ago

Except the Makefile here doesn't work on all platforms (e.g., Windows). When I ported Ryu to Mac and Windows, I didn't really have to modify the BUILD files, and it mostly just worked out of the box with Bazel.

I acknowledge that Bazel is a bit heavyweight, and requires an additional install that people would rather not have to go through. As such, I'm willing to have the Makefile be added here. However, I can't guarantee that it'll continue to work.

Please add a disclaimer to the top of the Makefile that this is a contributed file, and that it's maintained on a best-effort basis.

Finii commented 5 years ago

Adding at least the tests should not be too much work, right? And if you use this in another project you want the tests to be incorporated, no? Compiles on my machine is of course also an approach. :smiley_cat:

gritzko commented 5 years ago

Adding at least the tests should not be too much work, right?

That broadens the scope. The is a gtest dependency. My project has my own copy of gtest, there might or might not be system-default gtest... That increases the surface area. If ryu version is fixed by the hash, that might also be redundant. Basically, I may run tests on each supported platform, once. For strange cases, the are tests in my project, which effectively cover all ryu functions I use. More is not always better :)

ulfjack commented 5 years ago

The Makefile can contain rules to build as well as test it, and just not work for testing if gtest isn't there. Just sayin'. I'm happy to merge this without test support.

ulfjack commented 5 years ago

To be clear: I'm happy to merge this if you add a comment that this is contributed and support is best effort. It's ok not to have support for multiple platforms, or tests, for now.