Skip to content

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