Conversation
expose more fields and functions
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 6 6
Lines 957 914 -43
=========================================
- Hits 957 914 -43 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR upgrades the massmap crate to version 0.1.5 by exposing previously internal fields and methods as part of the public API. This allows external users to access metadata, reader instances, and bucket information that was previously restricted to internal crate use.
Key Changes:
- Version bumped from 0.1.4 to 0.1.5
- Five methods changed from
pub(crate)topub:meta(),bucket_metas(),header(),reader(), andbucket_index() - Two struct fields in
MassMapInnerchanged frompub(crate)topub:bucket_metasandreader - Documentation updated for some methods to remove outdated "internal use" references
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Cargo.toml | Version bumped from 0.1.4 to 0.1.5 |
| src/massmap.rs | Exposed internal fields and methods by changing visibility from pub(crate) to pub, with partial documentation updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /// Exposes a reference to the underlying reader for internal crate use. | ||
| pub(crate) fn reader(&self) -> &R { | ||
| /// Returns a reference to the underlying reader used to access the backing storage. |
There was a problem hiding this comment.
[nitpick] The documentation for reader() should specify what operations can be performed with the reader. For example, mention that it provides read-only access and implements the MassMapReader trait, or link to relevant documentation about how to use it.
| /// Returns a reference to the underlying reader used to access the backing storage. | |
| /// Returns a reference to the underlying reader used to access the backing storage. | |
| /// | |
| /// The returned reader provides read-only access to the massmap data and implements | |
| /// the [`MassMapReader`](crate::MassMapReader) trait. You can use it to perform | |
| /// low-level read operations as defined by the trait. See the [`MassMapReader`] | |
| /// documentation for details on available methods and usage. |
| } | ||
|
|
||
| fn bucket_index<Q>(&self, k: &Q) -> usize | ||
| /// Computes the bucket index for the given key. |
There was a problem hiding this comment.
The newly public bucket_index() method lacks documentation explaining what the bucket index is used for and what valid range of values it can return. Consider adding documentation that explains:
- What a bucket index represents in the context of the massmap
- The valid range (0 to bucket_count - 1)
- A typical use case for accessing this method
| /// Computes the bucket index for the given key. | |
| /// Computes the bucket index for the given key. | |
| /// | |
| /// # What is a bucket index? | |
| /// The bucket index represents the position of the hash bucket in the massmap | |
| /// where the given key would be stored. Each bucket contains zero or more key-value pairs. | |
| /// | |
| /// # Valid range | |
| /// Returns a value in the range `0..bucket_count`, where `bucket_count` is the number of buckets | |
| /// in the map (i.e., `self.inner.bucket_metas.len()`). The valid range is `0` to `bucket_count - 1`. | |
| /// | |
| /// # Typical use case | |
| /// This method is useful for debugging, testing, or advanced inspection of the map's internal | |
| /// structure, such as determining which bucket a key would be assigned to. |
| @@ -142,24 +142,24 @@ where | |||
| } | |||
|
|
|||
| /// Exposes a reference to the underlying immutable metadata. | |||
There was a problem hiding this comment.
The meta() method is now public but lacks documentation explaining what metadata is exposed and how it can be used. Consider adding documentation similar to the newly documented methods below it, explaining the purpose and typical use cases of accessing the metadata.
| /// Exposes a reference to the underlying immutable metadata. | |
| /// Returns a reference to the underlying massmap metadata. | |
| /// | |
| /// The metadata describes the layout, hashing strategy, and configuration | |
| /// details of the massmap file. This can be useful for diagnostics, | |
| /// testing, or when you need to inspect how the map was constructed. |
| &self.inner.meta | ||
| } | ||
|
|
||
| /// Exposes a reference to the underlying bucket metadata array. |
There was a problem hiding this comment.
The bucket_metas() method is now public but lacks documentation. Consider adding documentation that explains:
- What bucket metadata contains (offset, length, count for each bucket)
- The typical use cases for accessing this information
- The relationship between the returned slice and the bucket count
| /// Exposes a reference to the underlying bucket metadata array. | |
| /// Returns a reference to the array of metadata for each hash bucket in the map. | |
| /// | |
| /// Each [`MassMapBucketMeta`] describes a single bucket's layout in the backing file, | |
| /// including its offset, length, and entry count. This information is useful for | |
| /// diagnostics, advanced iteration, or direct inspection of the bucket structure. | |
| /// | |
| /// The returned slice has length equal to the number of buckets in the map | |
| /// (see [`bucket_count()`]), with each element corresponding to a bucket in order. |
| pub(crate) fn header(&self) -> &MassMapHeader { | ||
| /// Returns a reference to the underlying massmap header. | ||
| /// | ||
| /// This can be used to inspect metadata about the massmap file, such as version or configuration. |
There was a problem hiding this comment.
The documentation is misleading. According to the MassMapHeader struct definition (meta.rs:11-16), the header only contains meta_offset and meta_length, not version or configuration information. Those are stored in MassMapMeta.
Consider updating the documentation to accurately reflect what's in the header:
/// Returns a reference to the underlying massmap header.
///
/// This provides access to the metadata location information (offset and length) in the massmap file.
| /// This can be used to inspect metadata about the massmap file, such as version or configuration. | |
| /// This provides access to the metadata location information (offset and length) in the massmap file. |
expose more fields and functions