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.

Sign up for GitHub

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

Jump to bottom

Avoid NullReferenceException when accessing CustomOptions #6826

Merged
merged 1 commit into from
Nov 4, 2019
Merged

Avoid NullReferenceException when accessing CustomOptions #6826

merged 1 commit into from
Nov 4, 2019

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Oct 30, 2019

Even though the property is deprecated, it still shouldn't throw.

@jskeet
Avoid NullReferenceException when accessing CustomOptions
7282f29 
Even though the property is deprecated, it still shouldn't throw.
@jskeet jskeet requested a review from anandolee October 30, 2019 12:01
jtattermusch
jtattermusch approved these changes Oct 31, 2019
View reviewed changes
Copy link
Contributor

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jtattermusch
Copy link
Contributor

Does this need to be backported to 3.10.x or is it only problem on HEAD?

@jskeet
Copy link
Contributor Author

jskeet commented Oct 31, 2019

I'll double check - I think it's okay in 3.10.1.

@jskeet
Copy link
Contributor Author

jskeet commented Oct 31, 2019

Nope, it turns out it is a problem in 3.10.x.

@jtattermusch: Should I merge here, then create a separate PR that cherry-picks to the 3.10.x branch? What about #6824?

@ObsidianMinor ObsidianMinor mentioned this pull request Oct 31, 2019
Closed
@jtattermusch jtattermusch merged commit 61d75cb into protocolbuffers:master Nov 4, 2019
@jtattermusch
Copy link
Contributor

@jskeet can you please create a cherry-pick for 3.10.x?

@jtattermusch
Copy link
Contributor

@rafi-kamal there seems to be yet another problem with the 3.10.x release - what' the ETA for 3.11.x? (if it's not very soon, we should do another patch release).

jskeet added a commit to jskeet/protobuf that referenced this pull request Nov 4, 2019
@jskeet
Avoid NullReferenceException when accessing CustomOptions
4dd95d2 
(Note: the test is from protocolbuffers#6826, but the implementation is effectively reverting to an earlier one.)
@jskeet jskeet mentioned this pull request Nov 4, 2019
Merged
@rafi-kamal
Copy link
Contributor

@dankurka

I've seen an internal discussion about doing a release soon regarding CY, is this something that can go as part of that or does it need an immediate release?

I'm traveling now, if needed I'll be happy to do a minor release for this once I'm back on Wednesday. @TeBoring if you need any immediate release related help.

@jskeet jskeet deleted the fix-custom-options branch October 26, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtattermusch jtattermusch jtattermusch approved these changes

@ObsidianMinor ObsidianMinor ObsidianMinor approved these changes

@anandolee anandolee Awaiting requested review from anandolee

Assignees
No one assigned
Labels
bug c# cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jskeet @jtattermusch @rafi-kamal @ObsidianMinor @googlebot @protobuf-kokoro