-
Notifications
You must be signed in to change notification settings - Fork 757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rebuild with string_view argument, like the constructor #584
base: master
Are you sure you want to change the base?
Conversation
Could you add a test case with string literal like |
Sorry for all the back and forth. I thought the CI would run automatically when the PR was submitted. I didn't expect you to be my compiler. |
Thats ok. For some reason I have to approve after each commit. |
I'm a first-time contributor, so you need to approve the workflow. |
@gummif, the test you asked to add with
The test works with the constructor because of this implementation: // NOTE this constructor will include the null terminator
// when called with a string literal.
// An overload taking const char* can not be added because
// it would be preferred over this function and break compatiblity.
template<
class Char,
size_t N,
typename = typename std::enable_if<detail::is_char_type<Char>::value>::type>
ZMQ_DEPRECATED("from 4.7.0, use constructors taking iterators, (pointer, size) "
"or strings instead")
explicit message_t(const Char (&data)[N]) :
message_t(detail::ranges::begin(data), detail::ranges::end(data))
{
} I don't think this a good idea to add this for So, should I remove the test for string literal? Or you want me to test my new implementation for string_view with |
So what I was thinking is that currently there is only an overload for
How does that sound? |
More convenient, like the constructor
message_t(std::string_view str)
This is similar to PR #579 but with
std::string_view
instead ofstd::string&
.