protobuf
protobuf copied to clipboard
Set all but main project to nullable
Start with the periphery including test code.
@jskeet from #9801
Thanks - I'll look at this next week. We'll get there :)
I'm not 100% sure that starting at the periphery is going to be the most efficient approach though - because when we change the core later, all the test code will change too. Perhaps we should start with the inner-most code? (While the test project is null-oblivious, that won't affect anything. I'd suggest even for Google.Protobuf we could leave the project itself with nullability disabled, and enable it on a class-by-class basis until we've got a critical mass.)
Started this way simply to reduce later diffs: There are a lot of test code changes that did not need to be ported over from the other PR, but will be introduced incrementally once this goes in. This cuts the initial noise to make those PRs more focused on jus the related API changes.
(By which I mean after this PR, can start converting one or a few classes per PR and the diffs will just be related to those classes)
Okay, I see what you mean. Yup, that's fine. (There are multiple ways of approaching this kind of change.) It's on my to-do list for next week.
This is the wrong way around to do this. The main project should be made nullable first, then other projects. Otherwise, you'll need to react to all the nullable changes as main project is annotated.
@JamesNK: I don't think there's an objective "right" or "wrong" here - just different pros and cons.
I completely acknowledge that this way round means multiple iterations of the test projects - but I see that as a benefit. By doing it that way, you see the changes required in tests at the same time as the changes required in the main project, without any distractions from non-protobuf changes required for the tests.
That, to me, is a good way of getting a "sniff test" about whether the changes to the main project are appropriate - if the required changes to the tests seem reasonable, that's encouraging. It's a lot harder to pick those out when all the changes required to the tests come in the same commit.
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.
This PR is labeled inactive
because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it.
This PR was closed and archived because there has been no new activity in the 14 days since the inactive
label was added.