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

Use constexpr to define version numbers, not #define #107

Open
claremacrae opened this issue Feb 22, 2020 · 4 comments
Open

Use constexpr to define version numbers, not #define #107

claremacrae opened this issue Feb 22, 2020 · 4 comments
Labels
breaking_change Changes which will, or might, require users to change code on_hold

Comments

@claremacrae
Copy link
Collaborator

This arose out of #91.

It would be better to use constexpr instead of #define to define the version number components of this project in code - for type-safety.

Not everyone who uses this project uses it with CMake, so I would stick with our current structure - but I see that constexpr is a nicer - type-safe - way of setting the various version numbers...

I'll set up a separate issue to convert the #define approach in to constexpr instead...

Originally posted by @claremacrae in #91 (comment)

@claremacrae
Copy link
Collaborator Author

Current situation:

We use this:

def update_version_number_header(self):
pushdir(self.details.approval_tests_dir)
version_header = os.path.join("ApprovalTestsVersion.h")
text = \
F"""#ifndef APPROVALTESTS_CPP_APPROVALTESTSVERSION_H
#define APPROVALTESTS_CPP_APPROVALTESTSVERSION_H
#define APPROVALTESTS_VERSION_MAJOR {self.details.new_version_object['major']}
#define APPROVALTESTS_VERSION_MINOR {self.details.new_version_object['minor']}
#define APPROVALTESTS_VERSION_PATCH {self.details.new_version_object['patch']}
#define APPROVALTESTS_VERSION_STR "{version.get_version_without_v(self.details.new_version)}"
#define APPROVALTESTS_VERSION \\
(APPROVALTESTS_VERSION_MAJOR * 10000 + APPROVALTESTS_VERSION_MINOR * 100 + \\
APPROVALTESTS_VERSION_PATCH)
#endif //APPROVALTESTS_CPP_APPROVALTESTSVERSION_H
"""
write_file(version_header, text)

To generate this:

#ifndef APPROVALTESTS_CPP_APPROVALTESTSVERSION_H
#define APPROVALTESTS_CPP_APPROVALTESTSVERSION_H
#define APPROVALTESTS_VERSION_MAJOR 8
#define APPROVALTESTS_VERSION_MINOR 4
#define APPROVALTESTS_VERSION_PATCH 0
#define APPROVALTESTS_VERSION_STR "8.4.0"
#define APPROVALTESTS_VERSION \
(APPROVALTESTS_VERSION_MAJOR * 10000 + APPROVALTESTS_VERSION_MINOR * 100 + \
APPROVALTESTS_VERSION_PATCH)
#endif //APPROVALTESTS_CPP_APPROVALTESTSVERSION_H

@claremacrae
Copy link
Collaborator Author

claremacrae commented Feb 22, 2020

See the following template for the intention of this ticket...

https://github.com/jwillikers/cmake_configure_version_header/blob/fa644a6db6d61b7ac6dffce738bcb038fa4e6bd7/src/version.hpp.in#L1-L14

Instead of the @...@ text, our Python script would insert actual values.

We need to think about the use of a namespace, though - It would be a breaking change to add this now...

And we would use char[] instead of std::string_view

@claremacrae claremacrae added the breaking_change Changes which will, or might, require users to change code label Apr 26, 2020
@claremacrae
Copy link
Collaborator Author

We're marking this as on_hold, as no current plans to implement it...

@claremacrae
Copy link
Collaborator Author

It doesn't have to be a breaking change, if we add this as a new mechanism, and keep the old one...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking_change Changes which will, or might, require users to change code on_hold
Projects
None yet
Development

No branches or pull requests

1 participant