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

kvserver: move raft.proto to kvserverpb #76113

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

tbg
Copy link
Member

@tbg tbg commented Feb 5, 2022

kvserver: move raft.proto to kvserverpb

I'm starting to type code for #75729 and it's clear immediately
that we shouldn't have these protos in the kvserver package
as this will make dependency cycles hard to avoid.

We've long introduced the kvserverpb package, but simply
didn't pull all the protos into it.

This commit moves raft.proto to kvserverpb.

There's still a bit of protobuf left in api.proto, but
that can be handled separately (#76114)

Release note: None

@tbg tbg requested a review from a team as a code owner February 5, 2022 16:03
@tbg tbg requested a review from erikgrinaker February 5, 2022 16:03
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

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

I'm hoping we can eventually move these out of the kvserver domain entirely, but this seems like a good halfway house.

Glad to see you got some use out of your whiteboard!

I'm starting to type code for cockroachdb#75729 and it's clear immediately
that we shouldn't have these protos in the `kvserver` package
as this will make dependency cycles hard to avoid.

We've long introduced the `kvserverpb` package, but simply
didn't pull all the protos into it.

This commit moves `raft.proto` to `kvserverpb`.

There's still a bit of protobuf left in `api.proto`, but
that can be handled separately.

Release note: None
@tbg tbg force-pushed the confchangecontext branch from 23bb219 to 7051448 Compare February 5, 2022 19:28
@tbg
Copy link
Member Author

tbg commented Feb 5, 2022

bors r=erikgrinaker
TFTR!

@craig
Copy link
Contributor

craig bot commented Feb 5, 2022

Build succeeded:

@craig craig bot merged commit 40293d7 into cockroachdb:master Feb 5, 2022
@tbg tbg deleted the confchangecontext branch February 6, 2022 08:48
tbg added a commit to tbg/cockroach that referenced this pull request Feb 6, 2022
This is similar to cockroachdb#76113 in that it prevents import cycles in cockroachdb#75729.
We don't want the code moved here to necessarily stay in `kvserverbase`,
but that is a good place for now.

Release note: None
craig bot pushed a commit that referenced this pull request Feb 6, 2022
76120: kvserver: move raft version encoding to `kvserverbase` r=erikgrinaker a=tbg

This is similar to #76113 in that it prevents import cycles in #75729.
We don't want the code moved here to necessarily stay in `kvserverbase`,
but that is a good place for now.

Release note: None


Co-authored-by: Tobias Grieger <[email protected]>
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