Skip to content

Conversation

@ddelgrosso1
Copy link
Contributor

@ddelgrosso1 ddelgrosso1 commented Mar 21, 2025

This change is Reviewable

@ddelgrosso1 ddelgrosso1 requested a review from a team as a code owner March 21, 2025 19:10
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


google/cloud/internal/curl_options.h line 135 at r1 (raw file):

 * Sets the interface name to use as outgoing network interface.
 *
 * The default is to use whatever the TCP stack finds suitable.

Additionally, we may want to indicate that there's some formatting conventions available for the string per https://curl.se/libcurl/c/CURLOPT_INTERFACE.html


google/cloud/internal/curl_options.h line 137 at r1 (raw file):

 * The default is to use whatever the TCP stack finds suitable.
 */
struct Interface {

I'm assuming we want users to set this directly, so let's move this from the "internal" curl_options.h to google/cloud/rest_options.h.

Copy link
Contributor Author

@ddelgrosso1 ddelgrosso1 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 5 files reviewed, 2 unresolved discussions (waiting on @scotthart)


google/cloud/internal/curl_options.h line 135 at r1 (raw file):

Previously, scotthart (Scott Hart) wrote…

Additionally, we may want to indicate that there's some formatting conventions available for the string per https://curl.se/libcurl/c/CURLOPT_INTERFACE.html

Done.


google/cloud/internal/curl_options.h line 137 at r1 (raw file):

Previously, scotthart (Scott Hart) wrote…

I'm assuming we want users to set this directly, so let's move this from the "internal" curl_options.h to google/cloud/rest_options.h.

Done.

@codecov
Copy link

codecov bot commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.90%. Comparing base (b25d354) to head (60230e7).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
google/cloud/internal/curl_impl.cc 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15044      +/-   ##
==========================================
- Coverage   92.90%   92.90%   -0.01%     
==========================================
  Files        2351     2351              
  Lines      210211   210224      +13     
==========================================
+ Hits       195297   195301       +4     
- Misses      14914    14923       +9     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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 1 of 4 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ddelgrosso1)

@ddelgrosso1 ddelgrosso1 enabled auto-merge (squash) March 24, 2025 17:29
@ddelgrosso1
Copy link
Contributor Author

/gcbrun

@ddelgrosso1 ddelgrosso1 merged commit b63f69f into googleapis:main Mar 26, 2025
73 of 74 checks passed
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