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

roachpb: remove deprecated error fields #86683

Merged

Conversation

ajwerner
Copy link
Contributor

It turns out these unexported fields can cause pain. As far as I can tell,
these deprecated fields haven't been needed for a whole release. I'll note
that we probably ought to more generally fix gogoproto to not try to marshal
unexported fields and we should probably disallow them, but, alas, this is
the most expedient thing for me to do here.

Fixes #86664
Fixes #86509

Release justification: Important bug fix.

Release note: None

@ajwerner ajwerner requested a review from tbg August 23, 2022 18:06
@ajwerner ajwerner requested a review from a team as a code owner August 23, 2022 18:06
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the remove-deprecated-roachpb-error-fields branch from 3666d87 to 9793763 Compare August 23, 2022 19:42
@ajwerner
Copy link
Contributor Author

There's something not quite right here. I'm off until next Tuesday. If anybody finds time to do something about it between now and then, that'd be great.

@tbg
Copy link
Member

tbg commented Aug 29, 2022

Seeing this only now, let me see if I can figure it out in the hour I have left today.

@tbg tbg force-pushed the remove-deprecated-roachpb-error-fields branch from 9793763 to c2b9142 Compare August 29, 2022 15:32
@tbg
Copy link
Member

tbg commented Aug 29, 2022

Going to leave this sit now, the branch cut was delayed by a day so hopefully this will turn green and we can hit the button later today or early tmrw.

@tbg
Copy link
Member

tbg commented Aug 30, 2022

CI is behaving oddly, see slack

@tbg
Copy link
Member

tbg commented Aug 30, 2022

@ajwerner going to hand this back to you if that's ok. I think this PR should be green but somehow it isn't. Stuff passes locally. Hopefully the slack thread will produce something illuminating.

It turns out these unexported fields can cause pain. As far as I can tell,
these deprecated fields haven't been needed for a whole release. I'll note
that we probably ought to more generally fix gogoproto to not try to marshal
unexported fields and we should probably disallow them, but, alas, this is
the most expedient thing for me to do here.

Fixes cockroachdb#86664
Fixes cockroachdb#86509

Release justification: Important bug fix.

Release note: None
@ajwerner ajwerner force-pushed the remove-deprecated-roachpb-error-fields branch from 0d5e704 to 0c12f6c Compare August 31, 2022 01:09
@ajwerner
Copy link
Contributor Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 31, 2022

Build succeeded:

@craig craig bot merged commit 448352c into cockroachdb:master Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jsonpb: panic unmarshaling private field roachtest: schemachange/mixed-versions failed
3 participants