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

sql,stmtdiagnostics: remove some version gates #81558

Merged
merged 1 commit into from
May 20, 2022

Conversation

yuzefovich
Copy link
Member

This commit addresses several TODOs with my name on them about removing
the version gates, mostly around the conditional statement diagnostics
introduced in 22.1 cycle.

Release note: None

This commit addresses several TODOs with my name on them about removing
the version gates, mostly around the conditional statement diagnostics
introduced in 22.1 cycle.

Release note: None
@yuzefovich yuzefovich requested review from rharding6373, michae2 and a team May 19, 2022 23:13
@yuzefovich yuzefovich requested review from a team as code owners May 19, 2022 23:13
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@rharding6373 rharding6373 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)


pkg/sql/row/kv_batch_streamer.go line 30 at r1 (raw file):

// useStreamerEnabled determines whether the Streamer API should be used.
// TODO(yuzefovich): remove this in 23.1.

Was this bumped because the streamer is incomplete?

Copy link
Member Author

@yuzefovich yuzefovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @rharding6373)


pkg/sql/row/kv_batch_streamer.go line 30 at r1 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

Was this bumped because the streamer is incomplete?

Yeah, I think for sure we want to keep the setting for 22.2 release and then re-evaluate whether it is safe to have the streamer on by default without an escape hatch for 23.1 (maybe we'll remote the setting in 23.2, we'll see).

@yuzefovich
Copy link
Member Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented May 20, 2022

Build succeeded:

@craig craig bot merged commit 85d99af into cockroachdb:master May 20, 2022
@yuzefovich yuzefovich deleted the todos branch May 20, 2022 18:22
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.

3 participants