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

refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer #3512

Merged
merged 18 commits into from
Nov 8, 2023
Merged

refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer #3512

merged 18 commits into from
Nov 8, 2023

Conversation

caicancai
Copy link
Member

@caicancai caicancai commented Nov 7, 2023

Part of #3240

@caicancai caicancai reopened this Nov 8, 2023
@caicancai caicancai closed this Nov 8, 2023
@caicancai caicancai reopened this Nov 8, 2023
core/src/services/mod.rs Outdated Show resolved Hide resolved
@caicancai caicancai closed this Nov 8, 2023
@caicancai caicancai reopened this Nov 8, 2023
@caicancai caicancai closed this Nov 8, 2023
@caicancai caicancai reopened this Nov 8, 2023
@caicancai
Copy link
Member Author

I'm sorry that I've committed so many commits that take up CLI resources, I just started learning Rust, thinking that if cargo build is fine, it means that the syntax is fine, things don't seem to be what I thought, and I can only use CLI to correct my compilation errors

@suyanhanx
Copy link
Member

You don't have to close and open the PR constantly. A PR can be converted to a draft.
image

@caicancai
Copy link
Member Author

You don't have to close and open the PR constantly. A PR can be converted to a draft. image

Thank you for the reminder, next time I start to take such measures, it has been done so far, and it is time to review

@Xuanwo
Copy link
Member

Xuanwo commented Nov 8, 2023

if cargo build is fine, it means that the syntax is fine, things don't seem to be what I thought

Service TiKV is not enabled by default which means cargo build doesn't build the code you have changed. You can check your code via cargo clippy --features services-tikv.

@caicancai
Copy link
Member Author

if cargo build is fine, it means that the syntax is fine, things don't seem to be what I thought

Service TiKV is not enabled by default which means doesn't build the code you have changed. You can check your code via .cargo build``cargo clippy --features services-tikv

Thank you very much

@Xuanwo Xuanwo changed the title refactor(service/tikv): add tikvConfig to implement ConfigDeserializer refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer Nov 8, 2023
core/src/services/mod.rs Outdated Show resolved Hide resolved
core/src/services/tikv/backend.rs Show resolved Hide resolved
@caicancai
Copy link
Member Author

cc @Xuanwo

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thanks!

@Xuanwo Xuanwo merged commit 46f23cb into apache:main Nov 8, 2023
37 checks passed
@caicancai
Copy link
Member Author

caicancai commented Nov 8, 2023

@Xuanwo Hello, I find that this part of feature is inconsistent among different methods of backend.rs. This may be due to different developers. I would like to try to organize this part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants