Skip to content

cleanup: string_view and cast cleanups for Google import.#5139

Merged
htuch merged 5 commits intoenvoyproxy:masterfrom
htuch:error-code-2
Nov 28, 2018
Merged

cleanup: string_view and cast cleanups for Google import.#5139
htuch merged 5 commits intoenvoyproxy:masterfrom
htuch:error-code-2

Conversation

@htuch
Copy link
Member

@htuch htuch commented Nov 27, 2018

This mostly relates to changes in how protobuf Status is handled, which
will be reflected in future protobuf releases as well.

  • Replace a bunch of const std::string& with absl::string_view.

  • Static cast error_code() to uint64_t. Not needed in OSS today, but in
    the future this will be an enum class.

Risk Level: Low
Testing: bazel test //test/...

Signed-off-by: Harvey Tuch htuch@google.com

This mostly relates to changes in how protobuf Status is handled, which
will be reflected in future protobuf releases as well.

* Replace a bunch of const std::string& with absl::string_view.

* Static cast error_code() to uint64_t. Not needed in OSS today, but in
  the future this will be an enum class.

Risk Level: Low
Testing: bazel test //test/...

Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch htuch requested a review from lizan as a code owner November 27, 2018 22:10
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
lizan
lizan previously approved these changes Nov 28, 2018
Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

Great!

* Copy a string into the buffer.
* @param data supplies the string to copy.
*/
virtual void add(const std::string& data) PURE;
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove add(const void* data, uint64_t size) variant with this, just leave a TODO?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'll add TODOs. Too many call sites for this PR to cleanup (> 100).

* Set the header value by copying data into it.
*/
virtual void value(const std::string& value) PURE;
virtual void value(absl::string_view value) PURE;
Copy link
Member

Choose a reason for hiding this comment

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

same here for value(const char* value, uint32_t size)

@htuch htuch merged commit bff0167 into envoyproxy:master Nov 28, 2018
@htuch htuch deleted the error-code-2 branch November 28, 2018 17:44
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
…#5139)

This mostly relates to changes in how protobuf Status is handled, which
will be reflected in future protobuf releases as well.

* Replace a bunch of const std::string& with absl::string_view.

* Static cast error_code() to uint64_t. Not needed in OSS today, but in
  the future this will be an enum class.

Risk Level: Low
Testing: bazel test //test/...

Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
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.

3 participants