Skip to content
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

Make the MergeFrom method of type ReadOnlySequence<byte> public #11124

Closed
wants to merge 1 commit into from

Conversation

VAllens
Copy link
Contributor

@VAllens VAllens commented Dec 2, 2022

(Edited by jskeet) For public release notes:

Add a public IMessage.MergeFrom(ReadOnlySequence<byte>) extension method, exposing existing internal functionality.
(Parsing a completely new message from ReadOnlySequence<byte> was already publicly available, just not merging.)


Public the MergeFrom method of type ReadOnlySequence<byte>.

We found that the MergeFrom method of type ReadOnlySequence<byte> is internal.

We need it !!!

@google-cla
Copy link

google-cla bot commented Dec 2, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@jskeet
Copy link
Contributor

jskeet commented Dec 14, 2022

I'll need to carefully consider the impact of this on various versions etc. I'm still finishing up work from going on vacation, so I won't even be able to look at this until January. It may not make it into the next version; we'll see.

@VAllens
Copy link
Contributor Author

VAllens commented Dec 15, 2022

Great !!! This is positive.

@jskeet
Copy link
Contributor

jskeet commented Jan 4, 2023

Right, I've now had a better chance to evaluate this, and it looks fine. I've edited the PR description so that it'll be easier to create release notes.

@jskeet
Copy link
Contributor

jskeet commented Jan 4, 2023

@deannagarcia: This should be included in the next C# release. I'm happy for it to either stay here as a PR and get included via Copybara when that work is complete, or force-merged now. Let me know what you think.

@deannagarcia
Copy link
Member

Can you rebase this PR so we can get it submitted?

@jskeet
Copy link
Contributor

jskeet commented Jan 10, 2023

@deannagarcia: I may well be able to do that myself. Will give it a try.

@jskeet jskeet requested a review from a team as a code owner January 10, 2023 18:21
@jskeet jskeet requested review from jtattermusch and removed request for a team January 10, 2023 18:21
@jskeet
Copy link
Contributor

jskeet commented Jan 10, 2023

@deannagarcia: Done. Will run Kokoro as well...

@VAllens
Copy link
Contributor Author

VAllens commented Jan 11, 2023

@deannagarcia: I may well be able to do that myself. Will give it a try.

Thanks for @jskeet 's help. :)

@jskeet
Copy link
Contributor

jskeet commented Jan 19, 2023

I'm going to rebase and repush this PR so that it has a recent HEAD - I believe that we'll then be able to get this merged.

Public the MergeFrom method of type ReadOnlySequence<byte>
@jskeet jskeet changed the title Public the MergeFrom method of type ReadOnlySequence<byte> Make the MergeFrom method of type ReadOnlySequence<byte> public Jan 20, 2023
@jskeet
Copy link
Contributor

jskeet commented Jan 20, 2023

(Rebased again... I'm going to really try to get this all green today...)

@VAllens VAllens deleted the patch-1 branch January 30, 2023 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants