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

impl(generator): request_id-like helpers #13605

Merged
merged 3 commits into from
Feb 14, 2024

Conversation

coryan
Copy link
Contributor

@coryan coryan commented Feb 14, 2024

Add some functions to determine if a method has at least one request_id-like field, i.e., a non-repeating string field in the request message that expects UUIDV4 values.

Part of the work for #13595


This change is Reviewable

Add some functions to determine if a method has at least one
`request_id`-like field, i.e., a non-repeating string field in the
request message that expects UUIDV4 values.
Copy link

codecov bot commented Feb 14, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (8fa2180) 93.17% compared to head (95e1761) 93.17%.
Report is 2 commits behind head on main.

Files Patch % Lines
generator/internal/request_id.cc 94.11% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #13605    +/-   ##
========================================
  Coverage   93.17%   93.17%            
========================================
  Files        2221     2223     +2     
  Lines      192897   192998   +101     
========================================
+ Hits       179723   179819    +96     
- Misses      13174    13179     +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coryan coryan marked this pull request as ready for review February 14, 2024 02:13
@coryan coryan requested a review from a team as a code owner February 14, 2024 02:13
generator/internal/request_id.h Show resolved Hide resolved

std::string RequestIdFieldName(
YAML::Node const& service_config,
google::protobuf::MethodDescriptor const& descriptor) try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need #if GOOGLE_CLOUD_CPP_HAVE_EXCEPTIONS protecting exception code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. I have included an item for discussion in today's meeting, in case somebody has a reason to support builds without exceptions I have not thought of.

Basically the generator needs to compile on a limited number of builds (*San builds, clang-tidy builds, and the builds that use it). Running without exception support is not a requirement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me, but then we should probably remove the conditionals from generator/internal/printer.h.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #13611

generator/internal/request_id.cc Outdated Show resolved Hide resolved
generator/internal/request_id.cc Outdated Show resolved Hide resolved
generator/internal/request_id.cc Outdated Show resolved Hide resolved
generator/internal/request_id_test.cc Outdated Show resolved Hide resolved
generator/internal/request_id_test.cc Outdated Show resolved Hide resolved
generator/internal/request_id_test.cc Outdated Show resolved Hide resolved
@coryan coryan merged commit b82de9d into googleapis:main Feb 14, 2024
60 of 61 checks passed
@coryan coryan deleted the impl-generator-request-id-helpers branch February 14, 2024 20:47
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