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

Introduce a versioning system. #203

Merged
merged 5 commits into from
May 17, 2023
Merged

Introduce a versioning system. #203

merged 5 commits into from
May 17, 2023

Conversation

samcunliffe
Copy link
Member

@samcunliffe samcunliffe commented Dec 14, 2022

  • Add a mechanism for CMake to read a git hash and construct a "version" (release).
  • A tag of semver format on main (v1.2.3) is taken as an official version, otherwise it constructs a placeholder <branchname>_<tag or short format commit hash>.
    For example this commit has the "version" 201-versioning-in-cmake_b16cd31.
  • (Useful for debugging) you can also overwrite this with CMake configuration...
    cmake .. -DTDMS_VERSION=Ive_just_introduced_a_bug
    make install
    tdms --version
    

With a bit of luck I've explained all of this in the developers documentation 😸


Resolves #201.


Cheers @giordano and @alessandrofelder for suggestions. @giordano : if you have time and any opinions, they'd be appreciated.

Add a mechanism for CMake to read a git hash and construct a version. A
tag of semver format on main is taken as an official version, otherwise
construct a placeholder <branchname>_<tag or short format commit hash>.
@samcunliffe
Copy link
Member Author

samcunliffe commented Dec 14, 2022

It's a bit fiddly, but @willGraham01 : you can check out your local copy of main and merge this feature branch in (or cherry-pick the commit) to test.

Then git tag v100.99.98 (or whatever) and compile as normal, then check tdms --version.

@samcunliffe samcunliffe added enhancement New feature or request technical Technical and meta issues, not related to physics but infrastructure. ux Related to the user experience (CLI, workflow etc) opinions wanted Collaborators and users are encouraged to contribute their opinions housekeeping Code cleanup labels Dec 14, 2022
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

Looks overall good, but I believe you should handle the case where there is no git repository, which if I understand correctly (but haven't tried) may result in some errors when running CMake

tdms/cmake/version_from_git.cmake Show resolved Hide resolved
Copy link
Collaborator

@willGraham01 willGraham01 left a comment

Choose a reason for hiding this comment

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

It's a bit fiddly, but @willGraham01 : you can check out your local copy of main and merge this feature branch in (or cherry-pick the commit) to test.

Then git tag v100.99.98 (or whatever) and compile as normal, then check tdms --version.

Seems all good: cmake happily picks up if I'm sat at a tag, and failing that provides the auto-generated version string you specified. -DTDMS_version overrides both as predicted, although I made a comment about how adamant it is about doing this between builds.

tdms/CMakeLists.txt Show resolved Hide resolved
If the user grabs a snapshot tarball then there is no git repo so fall
emit a friendly warning message and fall back to the default release
name defined in globals.h.
samcunliffe added a commit that referenced this pull request May 2, 2023
If the user grabs a snapshot tarball then there is no git repo so
emit a friendly warning message and fall back to the default release
name defined in globals.h.
@codecov
Copy link

codecov bot commented May 15, 2023

Codecov Report

Patch coverage: 17% and project coverage change: -24 ⚠️

Comparison is base (11e9e8b) 48% compared to head (0e8e798) 24%.

Additional details and impacted files
@@          Coverage Diff          @@
##           main   #203     +/-   ##
=====================================
- Coverage    48%    24%    -24%     
=====================================
  Files        63     62      -1     
  Lines      7808   3593   -4215     
=====================================
- Hits       3760    874   -2886     
+ Misses     4048   2719   -1329     
Impacted Files Coverage Δ
tdms/src/argument_parser.cpp 69% <17%> (-5%) ⬇️

... and 54 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@samcunliffe samcunliffe disabled auto-merge May 15, 2023 17:41
doc/developers.md Outdated Show resolved Hide resolved
tdms/include/argument_parser.h Outdated Show resolved Hide resolved
tdms/tests/unit/test_argument_parser.cpp Show resolved Hide resolved
Co-authored-by: Will Graham <32364977+willGraham01@users.noreply.github.com>
@samcunliffe samcunliffe added this pull request to the merge queue May 17, 2023
Merged via the queue into main with commit bf6eaeb May 17, 2023
3 checks passed
@giordano giordano deleted the 201-versioning-in-cmake branch May 23, 2023 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request housekeeping Code cleanup opinions wanted Collaborators and users are encouraged to contribute their opinions technical Technical and meta issues, not related to physics but infrastructure. ux Related to the user experience (CLI, workflow etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Versioning
3 participants