-
Notifications
You must be signed in to change notification settings - Fork 377
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
fix: patch releases do not change the ABI #11499
fix: patch releases do not change the ABI #11499
Conversation
#define GOOGLE_CLOUD_CPP_VCONCAT(Ma, Mi, Pa) v##Ma##_##Mi | ||
#define GOOGLE_CLOUD_CPP_VEVAL(Ma, Mi, Pa) GOOGLE_CLOUD_CPP_VCONCAT(Ma, Mi, Pa) | ||
#define GOOGLE_CLOUD_CPP_NS \ | ||
GOOGLE_CLOUD_CPP_VEVAL(GOOGLE_CLOUD_CPP_VERSION_MAJOR, \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should also not pass GOOGLE_CLOUD_CPP_VERSION_PATCH
to GOOGLE_CLOUD_CPP_VEVAL
(and GOOGLE_CLOUD_CPP_VCONCAT
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps. <lawyerly>
In an abundance of caution I decided to keep the macros backwards compatible, because it is not clear if they are part of the public API or not.</lawerly>
. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack.
But while it might be a little hazy whether GOOGLE_CLOUD_CPP_NS
itself is public, hopefully VEVAL
and VCONCAT
are more clearly implementation details. We should have put "INTERNAL" in their names I guess.
In the end though, I'm happy with your abundance of caution if you're unconvinced.
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #11499 +/- ##
==========================================
- Coverage 93.81% 93.81% -0.01%
==========================================
Files 1808 1808
Lines 163469 163469
==========================================
- Hits 153360 153355 -5
- Misses 10109 10114 +5
☔ View full report in Codecov by Sentry. |
This change is