-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix: change Element's methods visibility #356
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces significant changes to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
grovedb/src/element/query.rs (1)
888-942
: Refactor test helper functions to align with new APIThe helper functions
get_query
andget_query_values
are recreated within the test module to preserve existing test cases after the removal of their public counterparts. Consider refactoring the tests to directly use the updated internal methods or expose necessary functionality through public interfaces. This would reduce code duplication and enhance maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
grovedb/src/element/delete.rs
(2 hunks)grovedb/src/element/exists.rs
(1 hunks)grovedb/src/element/get.rs
(6 hunks)grovedb/src/element/insert.rs
(12 hunks)grovedb/src/element/query.rs
(24 hunks)
🔇 Additional comments (22)
grovedb/src/element/query.rs (3)
Line range hint
235-318
: Verify external usage ofget_query_apply_function
The method
get_query_apply_function
has been changed topub(crate)
, restricting its visibility to within the crate. Please ensure that no external crates or modules depend on this function. If external access is required, consider providing a public wrapper or updating the documentation to reflect this change.
Line range hint
320-429
: Ensureget_path_query
visibility change does not affect external codeThe function
get_path_query
is nowpub(crate)
. Verify that this change does not impact any external components that might rely on this method. If external use is intended, consider maintaining its public visibility or offering alternative public methods.
861-863
: Check for external dependencies oniterator
methodThe visibility of the
iterator
method has been changed topub(crate)
. Ensure that no external code relies on this method. If it is part of the public API, you might need to retain its public visibility or provide an alternative interface.grovedb/src/element/exists.rs (1)
Line range hint
14-22
: Confirm internal sufficiency ofelement_at_key_already_exists
The method
element_at_key_already_exists
is nowpub(crate)
. Please verify that this change does not affect any external modules or crates relying on this function. If external access is necessary, consider providing a public interface.grovedb/src/element/delete.rs (3)
Line range hint
23-87
: Review the impact of removing the publicdelete
methodThe
delete
method has been removed. Ensure that this does not break any external dependencies. If external code relies on this method, consider providing an alternative public method or updating the documentation to inform users of the change.
Line range hint
23-87
: Assess the visibility change ofdelete_with_sectioned_removal_bytes
The method
delete_with_sectioned_removal_bytes
is nowpub(crate)
. Verify that this change aligns with the intended encapsulation and does not affect any external usage. If external access is required, consider exposing a public method or updating the API accordingly.
Line range hint
89-106
: Ensure no external dependence ondelete_into_batch_operations
Changing
delete_into_batch_operations
topub(crate)
restricts its usage to within the crate. Please confirm that no external modules depend on this function. If it needs to be accessible externally, consider keeping it public or providing an alternative.grovedb/src/element/get.rs (6)
Line range hint
33-63
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
64-103
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
104-126
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
127-317
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate. The version-specific implementations are well-handled.
Line range hint
318-355
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
356-399
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.grovedb/src/element/insert.rs (9)
Line range hint
22-79
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
80-122
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
123-171
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
172-209
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
210-261
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
262-296
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
297-352
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
353-399
: LGTM! Visibility change aligns with PR objectives.The visibility change from
pub
topub(crate)
helps limit the public API surface while maintaining proper functionality within the crate.
Line range hint
467-489
: LGTM! Test changes improve transaction handling.The changes properly handle transactions and ensure proper cleanup of resources while maintaining test coverage.
Changed visibility of several
Element
's methods to crate only and cleaned up those that became unused.Issue being fixed or feature implemented
GroveDb has huge public API, but in practice only a part shall be used by users. This PR is an attempt to address those visibility issues in
element
module.What was done?
Visibility of methods of
Element
except constructors was changed frompub
topub (crate)
, that revealed dead code that was deleted as well.How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Release Notes
Refactor
Element
implementation across different modulesChanges
delete.rs
,exists.rs
,get.rs
,insert.rs
, andquery.rs
have been changed frompub
topub(crate)