-
Notifications
You must be signed in to change notification settings - Fork 410
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Porting prost migration to raft 0.4 #204
Conversation
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
* Migrate to Rust 2018 * Remove rust-toolchain file The toolchains tested on CI are specified in their own ways, so this is only used for local builds where developers. Its better for developers not to have this file around in case they have their own preferences. (cherry picked from commit addddd2) Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
On master this is |
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.
LGTM
Seems it would be good if we made a branch off 0.4.1 and then merged this with that, then we can release 0.4.2 from that branch? |
@Hoverbear sounds good to me! |
Seems we never released 0.4.1, so we'll make this 0.4.1. :) |
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.
Rest LGTM, please fix some of the nits though :)
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
All comments addressed except those depends on tikv/protobuf-build#3. |
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
@nrc PTAL at the CLA |
I think this just needs the protobuf-build update and then can be merged |
Comments related to tikv/protobuf-build#3 are addressed. |
Failing tests now :-( |
Forcing a rebuild on CI |
I have one test failing locally but I didn't know why (at least seem to be unrelated to pb library). |
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
It's supposed to be fixed now. Let's wait for CI response... |
Still some errors, looks like lints |
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
All comments addressed. Mr. Travis CI, please start your performance |
|
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
I don't understand. I can see a lot of stacktraces in the nightly CI build, but it's saying test all passed. |
Yay! CI passed! |
@Hoverbear can we release this as 0.4.1 please? |
Sorry about the delay @ice1000 :( |
No problem, I can totally understand that you people are busy last week! |
This PR introduces API changes, and should be landed on 0.5.x instead of 0.4.x. |
@BusyJay Prior to 1.0.0 authors are free to break API as they please. 0.5.0 contains other changes and is already released. It's incompatible with TiKV. |
@BusyJay We have another PR that does the same thing but based on the master branch instead of the 0.4.0 release tag. |
Discussed on Slack, we agreed to yank 0.4.1 and use Prost via a Git dep for the time being |
For TiKV usages.
This is opened as a PR only for code review, please don't merge.
Currently there's one assertion error in tests but the production code are (seem to) working.