Closed DrLuke closed 2 years ago
@DrLuke Thanks again for the patch! I'm going to have a second look on it today or tomorrow, but I think we can merge this without any further modifications.
Please excuse my late reply again, I should block an hour or something a week for reviewing PRs on my open source repos.
I think using a String
here is fine, even though it involves some allocation. Otherwise a user might end up with lifetime problems, because you already said:
... it doesn't own it, thus the string passed to new must live at least as long as the OscAddress.
I'll merge this and publish a bugfix release. Thanks for the fix!
Might be better to do a minor release since the API changed slightly.
I agree. Thanks for reviewing and merging!
A little lifetime oversight of mine: The
OscAddress
needs to store the address as a string otherwise it doesn't own it, thus the string passed tonew
must live at least as long as theOscAddress
😄