feat: support deprecatedOnly option to make deprecated fields optional #1010
Conversation
tufandevrim
changed the title
tufandevrim
changed the title
|
@stephenh thoughts on this PR appreciated. I figured Right now in our codebase, we don't use |
| data?: Uint8Array | undefined; | ||
| repId: number[]; | ||
| /** @deprecated */ | ||
| repState: StateEnum[]; |
There was a problem hiding this comment.
Should this be repState? since it is deprecated? Maybe I am not understanding the purpose of the flag.
There was a problem hiding this comment.
It looks like there is a check to say that repeated fields should not be marked as optional
... @tufandevrim was there a reason to not include this for repeateds?
There was a problem hiding this comment.
@paralin @stephenh so sorry for the delay. My original plan was keeping repeated fields out of deprecatedOnly config but it really makes sense to make the repeated fields also optional if they are deprecated. I just updated the code, please review it when you get a chance. Thanks again for the review and feedback.
|
Hi @tufandevrim this seems pretty reasonable, see the one question about repeateds, but otherwise this lgtm! |
tufandevrim
force-pushed
the
deprecatedOnlyOptional
branch
from
cf33875
to
1252fd1
Compare
@stephenh yeah that was great feedback and made sense. Just updated the code to make deprecated repeated fields optional. Thanks again |
|
@tufandevrim looks like a minor prettier error in |
Oops! fixed now! |
|
Sweet, thanks @tufandevrim ! |
# [1.170.0](v1.169.1...v1.170.0) (2024-03-26) ### Features * support deprecatedOnly option to make deprecated fields optional ([#1010](#1010)) ([db23004](db23004))
|
🎉 This PR is included in version 1.170.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Add
deprecatedOnlyoption foruseOptionalsoption to make only deprecated fields optional. This PR aims to address the issue #993