chore(bigquery): migrate to std::optional#16210
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the use of absl::optional with std::optional across several files in the BigQuery v2 minimal internal library, aligning with C++17 standards. The reviewer suggests returning std::optional<std::chrono::system_clock::time_point> by value instead of by const& in ListJobsRequest because it is a small, trivially copyable type, which avoids pointer indirection and potential lifetime issues.
| std::optional<std::chrono::system_clock::time_point> const& | ||
| min_creation_time() const { | ||
| return min_creation_time_; | ||
| } | ||
| absl::optional<std::chrono::system_clock::time_point> const& | ||
| std::optional<std::chrono::system_clock::time_point> const& | ||
| max_creation_time() const { | ||
| return max_creation_time_; | ||
| } |
There was a problem hiding this comment.
Since std::chrono::system_clock::time_point is a small, trivially copyable type, std::optional<std::chrono::system_clock::time_point> is also small and cheap to copy. Returning it by value instead of by const& is more idiomatic in C++, avoids pointer indirection overhead, and prevents potential lifetime issues (dangling references) for callers.
std::optional<std::chrono::system_clock::time_point>
min_creation_time() const {
return min_creation_time_;
}
std::optional<std::chrono::system_clock::time_point>
max_creation_time() const {
return max_creation_time_;
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16210 +/- ##
==========================================
- Coverage 92.24% 92.24% -0.01%
==========================================
Files 2265 2265
Lines 210126 210126
==========================================
- Hits 193839 193828 -11
- Misses 16287 16298 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Replacing explicit usages of absl::optional with std::optional in the BigQuery Client library as part of broader modernization efforts