Access yaml-cpp version from within a program.#349
Access yaml-cpp version from within a program.#349samkellett wants to merge 2 commits intojbeder:masterfrom
Conversation
|
I'm uncertain that this is a positive change. In general, I'm hesitant to add source files that are generated by the build tool. What other major projects have this feature? |
|
It looks like boost writes the file manually (at least, the file version.hpp shows up directly in its downloaded tarball, and is not generated by the build tool). I'd slightly prefer that. I'm still not sure if I understand the use case here. What is the situation in which your unit test would fail? |
|
sorry i deleted that message as i wanted to provide a more thorough one On 11 January 2016 at 14:32, Jesse Beder notifications@github.com wrote:
|
|
Ah, I think understand (although I do not use travis myself, so my next question might be not possible). Why not, instead, use the same tools to build your development version as your prod or CI version? |
|
nar, travis doesn't make that so easy. it requires a yml file that ends up looking like a bastardised bash script (here's mine: https://github.com/galaxysoftware/galaxy/blob/master/.travis.yml). this can be solved by having all external dependencies in-tree and built with the product but this isn't that appealing w/r/t to scale over time |
|
FWIW, I'd strongly recommend having external deps in-tree. Makes for a very simple build system, and you don't get any versioning surprises. Anyways, what do you think about just writing version.h manually? |
|
yeah it's simpler for sure but i find it doesn't scale as well and can add to the build time unnecessarily. re: hand-made version.h yeah i'd be cool with that although i think the unit test should still draw the variables from cmake to verify that it's up-to-date with the rest of the project. i'd be happy to redo this patch that way if you like? |
|
Sure, that sounds good. |
Switched around the header and unit test. Now the unit test is created by CMake using the variables set in there and the version.h header is created manually. Unit test checks that both the CMake variables and header share the same version.
|
there you go. :) lemme know what you think |
|
Aren't pragmas ignored if they are not supported by the compiler? (making testing for And couldn't you practically stringify the three version macros for |
maybe i'm not sure. i based this header off of the existing files.
yeah you could. i'm happy to do it either way. |
|
I suggest stringifying for |
|
|
||
| #define YAML_CPP_VERSION_MAJOR 0 | ||
| #define YAML_CPP_VERSION_MINOR 5 | ||
| #define YAML_CPP_VERSION_PATCH 2 |
There was a problem hiding this comment.
You'll have to update this to 3 :)
There was a problem hiding this comment.
if that's the case then should the equivalent variable in the top-level CMakeLists.txt also be 3? especially as this is the variable that is used to generate the version_test that this file is checked against.
|
Sorry for the delay. Overall structure LGTM, just a few nits. Once you update them, please squash and rebase to HEAD. |
|
I would like this as well. Is there anything I can do to have it merge faster? |
5 participants
I have added the ability to get the yaml-cpp version via the new yaml-cpp/version.h header so that I can test that I am using the correct one (if travis gets out of sync with a dev computer for example). I have added tests for this.
I was unsure how you generated your include guards so I stuck with a hand-written one. Additionally I was unsure of whether version.h.in really lived inside include/yaml-cpp. I'm inclined to say that it isn't as that means that it will get installed along with the other headers. If you have any other suggestion I'll be happy to move it.
Cheers