-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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/schemachanger: reorder args on Build, clone nodes, minor renaming #66837
sql/schemachanger: reorder args on Build, clone nodes, minor renaming #66837
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 9 of 9 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 17 of 17 files at r2, 12 of 12 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)
pkg/sql/schemachanger/scbuild/builder.go, line 67 at r3 (raw file):
// outputNodes contains the internal state when building targets for an individual // statement. outputNodes scpb.State
Nit: Do we want to rename these to outputState?
pkg/sql/schemachanger/scbuild/builder.go, line 117 at r3 (raw file):
func Build( ctx context.Context, dependencies Dependencies, initial scpb.State, n tree.Statement, ) (outputNodes scpb.State, err error) {
Nit: Similar here
pkg/sql/schemachanger/scbuild/builder_test.go, line 97 at r3 (raw file):
stmts, err := parser.Parse(d.Input) require.NoError(t, err) var outputNodes scpb.State
Nit: rename
pkg/sql/schemachanger/scplan/plan_test.go, line 97 at r3 (raw file):
stmts, err := parser.Parse(d.Input) require.NoError(t, err) var outputNodes scpb.State
Nit: outputState?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)
d27186f
to
92dfe55
Compare
bors r+ |
Build failed (retrying...): |
bors r- |
Canceled. |
92dfe55
to
63c4c10
Compare
Release note: None
Release note: None
Release note: None
Release note: None
63c4c10
to
06dc96e
Compare
bors r=fqazi |
Build succeeded: |
Release note: None