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

types: remove no longer used version gate #77320

Closed
wants to merge 2 commits into from

Conversation

yuzefovich
Copy link
Member

@yuzefovich yuzefovich commented Mar 3, 2022

Depends on #77319.

This is done in preparation of breaking the dependency of sql/types on
roachpb (which is a part of the effort to clean up the dependencies of
execgen). Although there is an ask in place to not remove this code
even if it's not used at the moment (so that we have the necessary infra
in place if/when the need arises), in order to keep the functionality we
would need to pull out roachpb.Version out of roachpb, and I don't
want to embark on that since we're in stability period.

Addresses: #77234.

Release note: None

Release justification: low risk change to clean up the dependencies.

This commit moves the definition of `BytesEncodeFormat` enum from
`sessiondatapb` to `lex`. This is done in order to make `lex` not depend
on a lot of stuff (eventually on `roachpb`) and is a part of the effort
to clean up the dependencies of `execgen`. Note that the proto package
name is not changed, so this change is backwards-compatible.

Release note: None

Release justification: low risk change to clean up the dependencies.
This is done in preparation of breaking the dependency of `sql/types` on
`roachpb` (which is a part of the effort to clean up the dependencies of
`execgen`). Although there is an ask in place to not remove this code
even if it's not used at the moment (so that we have the necessary infra
in place if/when the need arises), in order to keep the functionality we
would need to pull out `roachpb.Version` out of `roachpb`, and I don't
want to embark on that since we're in stability period.

Release note: None

Release justification: low risk change to clean up the dependencies.
@yuzefovich yuzefovich requested a review from a team as a code owner March 3, 2022 04:59
@yuzefovich yuzefovich requested a review from a team March 3, 2022 04:59
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@yuzefovich
Copy link
Member Author

Note that the first commit is in a separate PR and should be ignored here.

@yuzefovich yuzefovich removed the request for review from a team March 3, 2022 05:02
@yuzefovich
Copy link
Member Author

Actually, looks like this is the last piece, so I'll continue this in #77260.

@yuzefovich yuzefovich closed this Mar 3, 2022
@yuzefovich yuzefovich deleted the execgen-4 branch March 3, 2022 05:23
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.

2 participants