Remove unnecessary TODO #335
Conversation
src/time_zone_name_win.cc
Outdated
| // TODO: Switch to std::string::data() when we require C++17 or higher. | ||
| len = static_cast<std::size_t>(::WideCharToMultiByte( | ||
| CP_UTF8, WC_ERR_INVALID_CHARS, ptr, chars_len, &result[0], | ||
| CP_UTF8, WC_ERR_INVALID_CHARS, ptr, chars_len, result.data(), |
There was a problem hiding this comment.
This lib seem to still reference older versions.
Yes, CCTZ version 2.X only requires C++11, so this has to stay as-is for now.
There was a problem hiding this comment.
perhaps, TODO should then be removed? Code ensures it's non empty, no need to switch to .data() imo
There was a problem hiding this comment.
Correct ... there is no need to switch to .data(). The contributor of this code just thought it would be nice when C++17 is required. Presumably you thought the same.
There was a problem hiding this comment.
should I remove this TODO, or simply close the PR? let me know
There was a problem hiding this comment.
I'm happy either way, so your choice.
pps83
changed the title
devbww
left a comment
devbww
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks.
We'll have to wait for owners @derekmauro or @dinord though.
|
It looks like our CI needs some maintenance. |
3 participants
Keep address-of first char for c++11 compatibility
Note, original pr was done against abseil, which is c++17 now. This lib seem to still reference older versions.