Skip to content

rust: rename items in linear reader #1341

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

Merged
merged 2 commits into from
Mar 3, 2025
Merged

Conversation

james-rms
Copy link
Collaborator

@james-rms james-rms commented Feb 27, 2025

Changelog

  • Breaking: renamed mcap::sans_io::read to mcap::sans_io::linear_reader.
  • Breaking: renamed mcap::sans_io::read::ReadAction to mcap::sans_io::linear_reader::LinearReadEvent
  • Breaking: renamed LinearReader::next_action() to LinearReader::next_event().
  • Breaking: renamed LinearReader::set_written() to LinearReader::notify_read().
  • Breaking: renamed mcap::tokio::read::RecordReader to mcap::tokio::linear_reader::LinearReader

Docs

Changes are reflected in cargo doc.

Description

This PR revises the names of functions and structs exposed from the sans_io module, because they're badly named as-is and we need to fix this now before we add more to this module.
Specifically:

  1. ReadAction gets renamed to LinearReadEvent. This is because there's no definition of "action" in this context that makes sense - NeedMore is not an action. NeedMore is requesting the caller take some action, but then GetMessage is not requesting the caller to do anything. "events" are a term more commonly used in other parsers such as SAX, and can be interpreted more generally.
  2. set_written() is renamed to notify_read(). This better reflects what it's for (notifying the reader of the result of a read), and I found the term written confusing on a struct meant to be used for reading.
  3. the tokio::RecordReader is renamed to LinearReader for consistency.
  4. The sans_io::read module is renamed to linear_reader - This better reflects its contents. When we add more readers to the sans_io module, I don't intend to add them all to the sans_io::read module, it doesn't help anyone to nest so deeply.
BeforeAfter

@james-rms james-rms merged commit b5f5562 into main Mar 3, 2025
23 checks passed
@james-rms james-rms deleted the jrms/rust-sans-io-renames branch March 3, 2025 20:11
bennetthardwick added a commit that referenced this pull request Mar 26, 2025
### Changelog
<!-- Write a one-sentence summary of the user-impacting change (API,
UI/UX, performance, etc) that could appear in a changelog. Write "None"
if there is no user-facing change -->

Make `Writher::with_options` public

### Docs

<!-- Link to a Docs PR, tracking ticket in Linear, OR write "None" if no
documentation changes are needed. -->

None

### Description

Makes the `Writer::with_options` method in the Rust library public to
easier support setting options.

This also bumps the version to 0.16.0 in preparation to release this and
the breaking changes from #1341.

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

Successfully merging this pull request may close these issues.

3 participants