feat(3284): remove partition-store dependency from wal-protocol #3435
Conversation
7fc356c to
241450f
Compare
|
@muhamadazmy ptal |
There was a problem hiding this comment.
Dear @lsytj0413, Thank you so much for taking care of this. It is definitely much better.
I only left only 2 comments to clean up few things. Then we can merge.
Another thing that you might have missed is cleaning up the dependencies in the partition-store/Cargo.toml for example to remove
[build-dependencies]
prost-build = { workspace = true }And any other prost dependencies that is no longer needed
Once those changes are done, we can proceed with the merge
| @@ -0,0 +1 @@ | |||
| pub mod protobuf_types; | |||
There was a problem hiding this comment.
Wondering why do we need the "partition_store" module. Can we instead include the protobuf_types.rs directly under the storage-api so it's storage_api::protobuf_types instead of storage_api::partition_store::protobuf_types?
| Completed, Free, Inboxed, Invoked, Suspended, | ||
| }; | ||
| use crate::protobuf_types::v1::invocation_status_v2::JournalTrimPoint; | ||
| use crate::protobuf_types::v1::journal_entry::completion_result::{ | ||
| use crate::partition_store::protobuf_types::v1::invocation_status_v2::JournalTrimPoint; |
There was a problem hiding this comment.
Maybe it's a good idea to clean up this. It's easier to read without this long prefix since it's a sub-module
| use crate::partition_store::protobuf_types::v1::invocation_status_v2::JournalTrimPoint; | |
| use invocation_status_v2::JournalTrimPoint; |
This suggestion applies to all sub-modules here
|
@muhamadazmy I have fixed most of it, expect below：
|
|
Thanks @lsytj0413 for the quick fix. That is totally fine. I wasn't sure if I will merge once the CI passes. Thanks again for your fine work :) |
fix: #3284