{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/7166","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/7166/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/7166/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/7166/events","html_url":"https://github.com/cockroachdb/cockroach/pull/7166","id":159755964,"number":7166,"title":"engine: move protos to (new) enginepb package","user":{"login":"tschottdorf","id":5076964,"avatar_url":"https://avatars.githubusercontent.com/u/5076964?v=3","gravatar_id":"","url":"https://api.github.com/users/tschottdorf","html_url":"https://github.com/tschottdorf","followers_url":"https://api.github.com/users/tschottdorf/followers","following_url":"https://api.github.com/users/tschottdorf/following{/other_user}","gists_url":"https://api.github.com/users/tschottdorf/gists{/gist_id}","starred_url":"https://api.github.com/users/tschottdorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tschottdorf/subscriptions","organizations_url":"https://api.github.com/users/tschottdorf/orgs","repos_url":"https://api.github.com/users/tschottdorf/repos","events_url":"https://api.github.com/users/tschottdorf/events{/privacy}","received_events_url":"https://api.github.com/users/tschottdorf/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"tamird","id":1535036,"avatar_url":"https://avatars.githubusercontent.com/u/1535036?v=3","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"milestone":null,"comments":6,"created_at":"2016-06-11T04:59:56Z","updated_at":"2016-06-14T17:58:11Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/7166","html_url":"https://github.com/cockroachdb/cockroach/pull/7166","diff_url":"https://github.com/cockroachdb/cockroach/pull/7166.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/7166.patch"},"body":"    Under leader-evaluated Raft, the MVCCStats delta will be computed before the\r\n    proposal. Since the delta only commits when the write set does, and since\r\n    updating the protobufs is sensitive to ordering (unlike using, for example,\r\n    increment operations on individual keys), the delta will be sent along with the\r\n    Raft command.\r\n\r\n    This necessitates adding a `MVCCStats` object to `roachpb.RaftCommand`, but the\r\n    references from `storage/engine` to `roachpb` in methods which sit on engine\r\n    protos make this circular.\r\n\r\n    This change shells out the engine protos into their own package instead.\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.io/review_button.svg\" height=\"35\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.io/reviews/cockroachdb/cockroach/7166)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/225963570","html_url":"https://github.com/cockroachdb/cockroach/pull/7166#issuecomment-225963570","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/7166","id":225963570,"user":{"login":"tschottdorf","id":5076964,"avatar_url":"https://avatars.githubusercontent.com/u/5076964?v=3","gravatar_id":"","url":"https://api.github.com/users/tschottdorf","html_url":"https://github.com/tschottdorf","followers_url":"https://api.github.com/users/tschottdorf/followers","following_url":"https://api.github.com/users/tschottdorf/following{/other_user}","gists_url":"https://api.github.com/users/tschottdorf/gists{/gist_id}","starred_url":"https://api.github.com/users/tschottdorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tschottdorf/subscriptions","organizations_url":"https://api.github.com/users/tschottdorf/orgs","repos_url":"https://api.github.com/users/tschottdorf/repos","events_url":"https://api.github.com/users/tschottdorf/events{/privacy}","received_events_url":"https://api.github.com/users/tschottdorf/received_events","type":"User","site_admin":false},"created_at":"2016-06-14T17:58:10Z","updated_at":"2016-06-14T17:58:10Z","body":"Yep, that should also have worked (though I find roachpb big and random enough already).\nComments addressed.\n\n---\n\nReview status: 0 of 129 files reviewed at latest revision, 10 unresolved discussions, some commit checks pending.\n\n---\n\n*[storage/replica.go, line 279 \\[r14\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKCF8lI5U8m5OW5T6xA:-KKEyT4uFQJCoU_t6H_L:807718035) ([raw file](https://github.com/cockroachdb/cockroach/blob/f6a2b4866881db3b8419e8b580378caa02701787/storage/replica.go#L279)):*\n<details><summary><i>Previously, bdarnell (Ben Darnell) wrote…</i></summary>\n> Why do we have two of these fields? (bad merge?)\n</details>\nExactly. Fixed.\n\n---\n\n*[storage/engine/engine_test.go, line 247 \\[r9\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEFvRVfwli6LBhgVdD:-KKEz-QBeLGRuX-hXNs1:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/66f16819d73f99495ea3819d3ff67a55fec430b0/storage/engine/engine_test.go#L247)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> nit: maybe make `m` a value =/\n</details>\nDone.\n\n---\n\n*[storage/engine/engine_test.go, line 302 \\[r9\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEFyx-kn1oAjTpsMYQ:-KKEz0mx7yZxdNy1GYMz:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/66f16819d73f99495ea3819d3ff67a55fec430b0/storage/engine/engine_test.go#L302)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> ditto\n</details>\nDone.\n\n---\n\n*[storage/engine/enginepb/mvcc.proto, line 45 \\[r13\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEJ76JxbS1AD3e9lO1:-KKEzRNyUKJGe1Lfd90A:397045135) ([raw file](https://github.com/cockroachdb/cockroach/blob/3db69b2692853dcb50bb2b34c977397264fc5a47/storage/engine/enginepb/mvcc.proto#L45)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> bummer. can you file an issue about moving roachpb.Key to keys?\n</details>\nDone: #7220\n\n---\n\n*[storage/engine/enginepb/mvcc.proto, line 166 \\[r13\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEJAZRHZ5tVJ1vGgS9:-KKEzg_0gKH2smTIN1i7:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/3db69b2692853dcb50bb2b34c977397264fc5a47/storage/engine/enginepb/mvcc.proto#L166)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> more random whitespace\n</details>\nDone.\n\n---\n\n*[storage/engine/enginepb/mvcc.proto, line 25 \\[r14\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKCGVOp61vL3EVaG_dT:-KKEzAq0B5vwYR3inUw1:-598250012) ([raw file](https://github.com/cockroachdb/cockroach/blob/f6a2b4866881db3b8419e8b580378caa02701787/storage/engine/enginepb/mvcc.proto#L25)):*\n<details><summary><i>Previously, bdarnell (Ben Darnell) wrote…</i></summary>\n> Hmm. It's most closely associated with `kv`, but a dependency chain of `roachpb -> enginepb -> kvpb` just feels wrong.\n</details>\nI agree. Could always move this into a top-level thing (which could also hold TxnMeta).\n\n---\n\n*[util/hlc/timestamp.proto, line 15 \\[r12\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEIVWUtjTK1leCH8zG:-KKEzlm9y37A2SMrC0Et:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/90b37a8d3c602dd3d36e1f22a7d0f378af7d0dc6/util/hlc/timestamp.proto#L15)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> might be more appropriate to put your own name here\n</details>\nDone.\n\n---\n\n*[util/hlc/timestamp.proto, line 38 \\[r12\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEITCL8hoZzdKROhQs:-KKEzvxOxoAOCkMQ7yJc:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/90b37a8d3c602dd3d36e1f22a7d0f378af7d0dc6/util/hlc/timestamp.proto#L38)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> random trailing newlines\n</details>\nDone.\n\n---\n\n*[util/hlc/timestamp_test.go, line 15 \\[r12\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-KKEIaWb5nQSJhWSuMq0:-KKEzzPSDQ6SV534oN1g:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/90b37a8d3c602dd3d36e1f22a7d0f378af7d0dc6/util/hlc/timestamp_test.go#L15)):*\n<details><summary><i>Previously, tamird (Tamir Duberstein) wrote…</i></summary>\n> ditto probably more likely you\n</details>\nDone.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cockroachdb/cockroach/7166#-:-KKF-4aeV59j7DrRNNU7:-2116926096)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"tschottdorf"}}