This is largely pretty straightforward. I left a comment on some of the more interesting bits.
result.append_range(Vector<wtf_size_t>(repetitions, index));I guess this would be a nice place to be able to use `std::views`...
boundary.append_range(base::span_from_cstring("boundary"));I think these should be spelled as `std::string_view("boundary")` but I didn't want to get into that too much here.
My general feeling is:
data.append_range(JpegImage());This looks a bit silly but is a straightforward translation of the existing code.
It's because we're converting between a vector of unsigned char to a vector of signed char...
types.append_range(kAllEvents);This seemed a bit nicer than creating the temporary Vector but LMK if you don't like this.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
static constexpr std::string_view kAllEvents[] = {Blink Style Guide: Prefer blink:: types over STL and base types. Consider using 'const char*' or 'StringView' instead of 'std::string_view' for the array of string literals.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
lgtm, though you might want to split this if you don't want to spend time in rebase hell 😊
boundary.append_range(base::span_from_cstring("boundary"));I think these should be spelled as `std::string_view("boundary")` but I didn't want to get into that too much here.
My general feeling is:
- it's probably generally undesirable to implicitly treat classic NUL-terminated C string literals as ranges
- code that doesn't want the NUL can use `std::string_view`
- ... code that does want the NUL can use... something else.
Acknowledged
static constexpr std::string_view kAllEvents[] = {Blink Style Guide: Prefer blink:: types over STL and base types. Consider using 'const char*' or 'StringView' instead of 'std::string_view' for the array of string literals.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
OK but won't fix: `std::string_view` makes more sense here.
Migrate Vector::AppendVector()/Vector::AppendSpan() to append_range()
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |