Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add volk_version.h #346

Merged
merged 1 commit into from
Feb 16, 2020
Merged

add volk_version.h #346

merged 1 commit into from
Feb 16, 2020

Conversation

michaelld
Copy link
Contributor

@michaelld michaelld commented Feb 9, 2020

subject says it all. standard CMake substitutions. resulting VOLK_VERSION will be in the format MAMIMT: MA == major; MI == minor; MT == maint; any will have a preceding 0 if the number is otherwise a single digit (e.g., MA of "2" will result in "02" here, but "11" will just be "11"). This is the same style as done by Boost and other projects, since it makes for simple macro-based comparisons in C / C++ code.

Closes: #338

@michaelld
Copy link
Contributor Author

This change should be sufficient for GR, e.g., for the broken rotator in 2.0 that we fixed in 2.1.

Copy link
Contributor

@jdemel jdemel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks good. But there are a few minor things.
Mostly, the question arises if volk_version.h should be included in volk.h. volk_malloc.h etc. are included and I'd prefer to add this header as well.

include/volk/volk_version.h.in Outdated Show resolved Hide resolved
@michaelld
Copy link
Contributor Author

Mostly, the question arises if volk_version.h should be included in volk.h. volk_malloc.h etc. are included and I'd prefer to add this header as well.

I had thought this too, but decided to not for the initial PR. I think since volk.h includes all of the headers except volk_vector.hh, it should also include this one. I'll add it.

@michaelld
Copy link
Contributor Author

volk_version.h is now included in volk.h

@michaelld
Copy link
Contributor Author

@marcusmueller do you think the PR's addition (volk_version.h) is sufficient for what GR needs, per our recent discussions?

Copy link
Contributor

@jdemel jdemel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michaelld
Copy link
Contributor Author

Hoping to get an approval from at least 1 GR dev here ... so I tagged a few!

@jdemel jdemel merged commit 715ed1e into gnuradio:master Feb 16, 2020
Alesha72003 pushed a commit to Alesha72003/volk that referenced this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add volk_version.h
3 participants