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: use Latest[Non]PrimaryIndexDescriptorVersion constants in more places #72818

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

mgartner
Copy link
Collaborator

@mgartner mgartner commented Nov 16, 2021

These constants have been moved from tabledesc to descpb to avoid import
cycles.

Release note: None

@mgartner mgartner requested review from ajwerner, postamar and a team November 16, 2021 15:37
@mgartner mgartner requested a review from a team as a code owner November 16, 2021 15:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@ajwerner ajwerner 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 23 of 23 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @postamar)


pkg/sql/doctor/doctor_test.go, line 56 at r1 (raw file):

				Version:             descpb.PrimaryIndexWithStoredColumnsVersion,

This one I could see wanting to leave as the fixed value. 🤷

@mgartner mgartner force-pushed the use-latest-index-version-consts branch 2 times, most recently from a12a16a to 0ee2fe2 Compare November 16, 2021 16:28
Copy link
Collaborator Author

@mgartner mgartner 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! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner and @postamar)


pkg/sql/doctor/doctor_test.go, line 56 at r1 (raw file):

Previously, ajwerner wrote…
				Version:             descpb.PrimaryIndexWithStoredColumnsVersion,

This one I could see wanting to leave as the fixed value. 🤷

Done.

@mgartner mgartner force-pushed the use-latest-index-version-consts branch 2 times, most recently from 2a01881 to 57f5061 Compare December 10, 2021 00:05
@mgartner
Copy link
Collaborator Author

@ajwerner mind taking another quick look? I had to move the constants to avoid a cycle.

Copy link
Contributor

@postamar postamar left a comment

Choose a reason for hiding this comment

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

All good!

@mgartner mgartner force-pushed the use-latest-index-version-consts branch from 57f5061 to 0b9db6d Compare December 10, 2021 01:35
…laces

These constants have been moved from tabledesc to descpb to avoid import
cycles.

Release note: None
@mgartner mgartner force-pushed the use-latest-index-version-consts branch from 0b9db6d to 08435c3 Compare December 13, 2021 15:53
@mgartner
Copy link
Collaborator Author

TFTRs!

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 13, 2021

Build succeeded:

@craig craig bot merged commit d411bd0 into cockroachdb:master Dec 13, 2021
@mgartner mgartner deleted the use-latest-index-version-consts branch December 21, 2021 16:00
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.

4 participants