Skip to content

distsqlrun: restructure a snippet of gnarly code#32368

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
andreimatei:distsql-txn
Nov 15, 2018
Merged

distsqlrun: restructure a snippet of gnarly code#32368
craig[bot] merged 1 commit intocockroachdb:masterfrom
andreimatei:distsql-txn

Conversation

@andreimatei
Copy link
Copy Markdown
Contributor

@andreimatei andreimatei commented Nov 14, 2018

Release note: None

@andreimatei andreimatei requested review from a team November 14, 2018 23:25
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Nov 14, 2018

This change is Reviewable

nvb
nvb suggested changes Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

pkg/sql/distsqlrun/processors.go, line 683 at r1 (raw file):

		}
		if meta != nil {
			if meta.Err != nil {

What's up with this?

andreimatei
andreimatei commented Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei 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

pkg/sql/distsqlrun/processors.go, line 683 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

What's up with this?

sorry, that came from the future, a different timeline. Removed.

nvb
nvb approved these changes Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@nvb nvb left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

pkg/sql/distsqlrun/processors.go, line 683 at r1 (raw file):

Previously, andreimatei (Andrei Matei) wrote…

sorry, that came from the future, a different timeline. Removed.

This is a different object, so linearizability still holds.

jordanlewis
jordanlewis approved these changes Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jordanlewis jordanlewis left a comment

Choose a reason for hiding this comment

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

Thanks, Andrei. :lgtm:

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained

andreimatei
andreimatei commented Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

bors r+

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained

andreimatei
andreimatei commented Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

bors r-

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 15, 2018

Canceled

andreimatei
andreimatei commented Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

bors r+

Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 15, 2018

Build failed (retrying...)

@nvb nvb mentioned this pull request Nov 15, 2018
Merged
@nvb
Copy link
Copy Markdown
Contributor

nvb commented Nov 15, 2018

bors r-

Looks like this needs a make protobuf before it can be merged.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 15, 2018

Canceled

andreimatei
andreimatei commented Nov 15, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 2 stale)

craig bot pushed a commit that referenced this pull request Nov 15, 2018
@andreimatei
Merge #32368
60fc55f 
32368: distsqlrun: restructure a snippet of gnarly code r=andreimatei a=andreimatei

Release note: None

Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 15, 2018

Build succeeded

@craig craig bot merged commit dd03b8c into cockroachdb:master Nov 15, 2018
@andreimatei andreimatei deleted the distsql-txn branch November 15, 2018 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jordanlewis jordanlewis jordanlewis approved these changes

+1 more reviewer

@nvb nvb nvb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jordanlewis jordanlewis

@nvb nvb

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andreimatei @cockroach-teamcity @nvb @jordanlewis