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 preprocessor macros to retrieve versions #258

Closed
wants to merge 1 commit into from

Conversation

SanghyunKo
Copy link

BEGINRELEASENOTES

  • Add preprocessor macros to retrieve versions

ENDRELEASENOTES

I've realized that retrieving versions for preprocessing is somehow quite tricky in Podio. Adding a preprocessor macro would make it more helpful to developers (support backward compatibility for lcg releases, .etc). I believe that the macro will be okay as long as we maintain major.minor.patch convention for the version (mimicked how ROOT does for versioning [link]), but please let me know if there is any kind of issue you concern. Thanks.

@tmadlener
Copy link
Collaborator

Hi Sanghyun,
thanks for this. I agree that this is quite cumbersome at the moment. There is a very similar pull request already: #238

I think the preprocessor macros are almost identical (apart from using 16 instead of 8 bits). Additionally that would also offer a Version class that can be used at runtime. Maybe you could check if something is missing there for your purposes?

@SanghyunKo
Copy link
Author

Hi @tmadlener, thanks for the comment. Indeed, I didn't notice there is already a similar PR for this. It looks more than enough for me already (and more ambitious in the sense of controlling the file's version, not just only the library's version). I'll close this PR.

The last question from my side is - do we target to merge #238 for v00-15? I hope so since this will be quite useful when we introduce MutableClass (#205) for the next release.

@SanghyunKo SanghyunKo closed this Feb 3, 2022
@tmadlener
Copy link
Collaborator

The last question from my side is - do we target to merge #238 for v00-15? I hope so since this will be quite useful when we introduce MutableClass (#205) for the next release.

Yes, I think we definitely aim for that.

@SanghyunKo
Copy link
Author

Thanks @tmadlener!

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.

2 participants