Skip to content

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

Merged
Xuanwo merged 18 commits intoapache:mainfrom
caicancai:tikv
Nov 8, 2023
Merged

refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer#3512
Xuanwo merged 18 commits intoapache:mainfrom
caicancai:tikv

Conversation

@caicancai
Copy link
Copy Markdown
Member

@caicancai caicancai commented Nov 7, 2023

Part of #3240

@caicancai caicancai requested a review from Xuanwo as a code owner November 7, 2023 14:42
@github-actions github-actions Bot added the releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" label Nov 7, 2023
@caicancai caicancai closed this Nov 7, 2023
@caicancai caicancai reopened this Nov 8, 2023
@caicancai caicancai closed this Nov 8, 2023
@caicancai caicancai reopened this Nov 8, 2023
Comment thread core/src/services/mod.rs
@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
Copy Markdown
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
Copy Markdown
Member

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

@caicancai
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Comment thread core/src/services/mod.rs Outdated
Comment thread core/src/services/tikv/backend.rs
Comment thread core/src/services/tikv/backend.rs Outdated
@caicancai
Copy link
Copy Markdown
Member Author

cc @Xuanwo

Copy link
Copy Markdown
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
@caicancai
Copy link
Copy Markdown
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

Labels

releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants