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

feat : Implementing config for part of services #4344

Merged
merged 12 commits into from
Mar 13, 2024
Merged

Conversation

AnuRage-git
Copy link
Contributor

Have implemented XxxConfig for /core/src/services/memory as per the issue Tracking issues of RFC-3197: Config #3240. Requesting review.

Requesting a review of the PR. I have added the functionalities as I have understood from the described issue.
feat : Implement config for module in services
backend.rs Outdated Show resolved Hide resolved
@dqhl76
Copy link
Member

dqhl76 commented Mar 12, 2024

Please solve the CI problem (cargo fmt) and expose config in core/src/services/mod.rs (you can take this as an example)

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 a lot for your contribution, here are some suggestions for this PR. Wish you have fun here.

core/src/services/memory/backend.rs Outdated Show resolved Hide resolved
core/src/services/memory/backend.rs Outdated Show resolved Hide resolved
core/src/services/memory/backend.rs Show resolved Hide resolved
@xyjixyjixyji
Copy link
Contributor

You can directly run cargo fmt under the core directory to format all files under it.

Besides, you need to make clippy happy by fixing the warning emitted by cargo clippy 😀

@AnuRage-git
Copy link
Contributor Author

@Ji-Xinyou Done. All checks have passed. I was new to Rust so it took some time for me to understand cargo tool functionalities

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 02437f8 into apache:main Mar 13, 2024
54 checks passed
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.

4 participants