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

Added a margo-version.h header generated #252

Merged
merged 1 commit into from
May 11, 2023
Merged

Conversation

mdorier
Copy link
Contributor

@mdorier mdorier commented Mar 29, 2023

Adds a margo-version.h header generated from the version found in configure.ac. This is needed if we want libraries built on top of Margo to rely on different functions depending on their availability.

@carns Can you double-check that I haven't done anything stupid?

@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Merging #252 (4096f4b) into main (3b8d8f8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #252   +/-   ##
=======================================
  Coverage   54.40%   54.40%           
=======================================
  Files          65       65           
  Lines        8865     8865           
  Branches     1164     1164           
=======================================
  Hits         4823     4823           
  Misses       3345     3345           
  Partials      697      697           
Impacted Files Coverage Δ
include/margo.h 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mdorier mdorier requested a review from carns March 29, 2023 19:08
AC_INIT([margo], [0.13.1], [],[],[])

MARGO_VERSION_MAJOR=0
Copy link
Member

Choose a reason for hiding this comment

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

I like having a version.h. I think it might be possible with an m4 macro to reuse one version number definition though, to make sure we don't accidentally get them out of sync. I'll tinker a little and report back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Argobots is a good place to look for this. I was thinking of doing it but got side-tracked looking into ways to get the version from git. Feel free to change this PR, I also don't really like that the version number is in two places (even though they are close by).

@mdorier
Copy link
Contributor Author

mdorier commented May 10, 2023

@carns fine if I merge this? We can circle back to it and try to have m4 get the version number from a file later.

Copy link
Member

@carns carns left a comment

Choose a reason for hiding this comment

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

Yeah this is fine, I opened #257 to track the later cleanup.

I actually got distracted looking at this because of the unit test failures due to one-way self rpc timing, no need to hold this up though.

@mdorier mdorier merged commit bed7b0b into main May 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants