Skip to content

Conversation

@evalon32
Copy link
Contributor

@evalon32 evalon32 commented Nov 7, 2024

Protobuf 6.30.0 will change the return types of Descriptor::name() and other methods to absl::string_view. This makes the code work both before and after such a change.


This change is Reviewable

Protobuf 6.30.0 will change the return types of Descriptor::name() and other methods to absl::string_view.
This makes the code work both before and after such a change.
@evalon32 evalon32 requested a review from a team as a code owner November 7, 2024 15:59
Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @evalon32)


google/cloud/spanner/internal/status_utils.cc line 19 at r1 (raw file):

#include "google/cloud/internal/status_payload_keys.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"

Please use our wrapper for this header "google/cloud/internal/absl_str_cat_quiet.h"


google/cloud/spanner/testing/status_utils.cc line 17 at r1 (raw file):

#include "google/cloud/spanner/testing/status_utils.h"
#include "google/cloud/grpc_error_delegate.h"
#include "absl/strings/str_cat.h"

Please use our wrapper for this header "google/cloud/internal/absl_str_cat_quiet.h"

@scotthart
Copy link
Member

/gcbrun

Copy link
Contributor Author

@evalon32 evalon32 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @scotthart)


google/cloud/spanner/internal/status_utils.cc line 19 at r1 (raw file):

Previously, scotthart (Scott Hart) wrote…

Please use our wrapper for this header "google/cloud/internal/absl_str_cat_quiet.h"

Done.


google/cloud/spanner/testing/status_utils.cc line 17 at r1 (raw file):

Previously, scotthart (Scott Hart) wrote…

Please use our wrapper for this header "google/cloud/internal/absl_str_cat_quiet.h"

Done.

@scotthart
Copy link
Member

/gcbrun

@codecov
Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.07%. Comparing base (291074b) to head (92f1208).
Report is 14 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14828   +/-   ##
=======================================
  Coverage   93.06%   93.07%           
=======================================
  Files        2319     2319           
  Lines      209023   209026    +3     
=======================================
+ Hits       194535   194545   +10     
+ Misses      14488    14481    -7     

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

ProtoEnum::GetTypeName() and ProtoMessage::GetTypeName() now have
the same return type as before.
@evalon32 evalon32 changed the title Prepare code for breaking change in Protobuf C++ API. feat: prepare for breaking change in Protobuf C++ API. Nov 7, 2024
@evalon32 evalon32 requested a review from scotthart November 8, 2024 17:04
@scotthart
Copy link
Member

We're going to wait on merging these changed until we are closer to updating our dependencies to include protobuf v30.

@scotthart scotthart added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 14, 2024
@scotthart
Copy link
Member

/gcbrun

@scotthart
Copy link
Member

/gcbrun

@scotthart scotthart removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 14, 2024
Copy link
Member

@scotthart scotthart left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @evalon32)

@scotthart scotthart merged commit 04f67a7 into googleapis:main Nov 14, 2024
75 checks passed
@evalon32 evalon32 deleted the proto-string-view-return branch November 14, 2024 21:08
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