OneOf Enum + Optional #912
Conversation
mmillerbe
commented
May 6, 2022
mmillerbe
commented
- Added BuildTest for OneOfEnum + Optional field
- Updated System.Collections.Immutable package version
- CodeGenerator: check IsProto3OptionalSyntheticOneOf before generating OneOfEnum
- CodeGenerator.OneOfStub: added _anyProto3Optional field; added IsProto3OptionalSyntheticOneOf method
|
I had a look - sorry for delay; I don't think we need to explicit field tracking - pretty sure we can infer that from the ambient state; see 52063df - and; locally at least, I had problems with the tests not working - unrelated to your changes; I had to fix a few things - see 16568c1 I don't think I can push to your remote; any chance you can absorb the above two, so I don't screw up credit? |
|
Thanks very much for looking! Integrating your changes now. |
|
Hey, I'm sorry Marc! Your changes look good to me, but I'm a little confused on how to absorb your changes. I see that you pushed to the mmillerbe-main branch of this repo, but I don't have that branch in my fork, and I'm a little confused as to how to pull it down. It's definitely fine with me if you just want to merge this as-is. |
|
Last comment from me today, Marc! I pulled the changes you made in mmillerbe-main into the mmillerbe fork's main. Let me know if I did that incorrectly, and apologies in advance if I did! |
|
I'll figure it out next week; thanks
…On Fri, 26 Aug 2022, 15:01 mmillerbe, ***@***.***> wrote:
Last comment from me today, Marc! I pulled the changes you made in
mmillerbe-main into the mmillerbe fork's main. Let me know if I did that
incorrectly, and apologies in advance if I did!
—
Reply to this email directly, view it on GitHub
<#912 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMFAYDFXFGNSFLOJS53V3DE4PANCNFSM5VGWKSJA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Thanks! Apologies if I made more work there. Mike |
|
No, it is fine. I appreciate your input!
…On Sat, 27 Aug 2022, 14:26 mmillerbe, ***@***.***> wrote:
Thanks! Apologies if I made more work there.
Mike
—
Reply to this email directly, view it on GitHub
<#912 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEHMFQ7WTLMVAKN74WLN3V3IJORANCNFSM5VGWKSJA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Hey Marc, Apologies for continuing to bug you about this! Just wanted to see if there's anything I could help out with here. Thank you! |
|
Hey Marc, Just wanted to bump this - any chance of getting it merged in? I'm happy to work through anything that'd make it easier. Thanks again! |
|
The bump is justified; I'll try to take a look - sorry for delay, but I have at least been busy in this area! |
|
No need to apologize - I appreciate all the work you do on this! Thanks again! |
|
Hey Marc, Hope you're doing well! Just wanted to check on this. Could we get this (or something like it) merged in? Or any chance this has been resolved via another change? Thanks! |
|
change looks good (sorry that I'm not keeping up); merge hell, though - might be worth yanking into a clean branch from |
|
Thanks! I will work on getting you a clean PR |
|
Hey Mark, I created this PR #1132 just cherry-picking the commit you referenced above. I did create a few files / directories that I think might be out of date with the current directory structure. Let me know if you want me to delete or move those. Oh, and I wasn't sure if you wanted me to also pull in 52063df that you added to avoid the field. Happy to do that as well. Thanks again! |
