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

Provide version major/minor/patch macros #2014

Merged
merged 4 commits into from
Mar 4, 2023

Conversation

marcalff
Copy link
Member

@marcalff marcalff commented Mar 3, 2023

Fixes #2012

Changes

Added the following macros:

  • OPENTELEMETRY_VERSION_MAJOR
  • OPENTELEMETRY_VERSION_MINOR
  • OPENTELEMETRY_VERSION_PATCH

This is helpful for user code, to support multiple versions in case of incompatible changes.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@marcalff marcalff requested a review from a team March 3, 2023 21:27
@codecov
Copy link

codecov bot commented Mar 3, 2023

Codecov Report

Merging #2014 (0b626b7) into main (4dff60a) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2014   +/-   ##
=======================================
  Coverage   87.32%   87.32%           
=======================================
  Files         166      166           
  Lines        4673     4673           
=======================================
  Hits         4080     4080           
  Misses        593      593           

Copy link
Member

@esigo esigo left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks for the PR :)

@esigo esigo added the ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve) label Mar 3, 2023
@@ -8,6 +8,10 @@

#define OPENTELEMETRY_ABI_VERSION_NO 1
#define OPENTELEMETRY_VERSION "1.8.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Use # operator for preprocessor to define OPENTELEMETRY_VERSION with the new VERSION macros?

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically possible, but the resulting code, with OPENTELEMETRY_CONCAT() and OPENTELEMETRY_STRINGIFY(), will be extremely obfuscated.

There is no risk that the string version goes out of sync with major/minor/patch versions, there is a unit test to catch this.

@lalitb lalitb enabled auto-merge (squash) March 3, 2023 23:33
@lalitb lalitb merged commit 649829f into open-telemetry:main Mar 4, 2023
@marcalff marcalff deleted the fix_api_version_2012 branch July 4, 2023 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide major/minor/patch version macros
4 participants