Skip to content

Fix clippy and rustfmt errors#3

Merged
dylanmc merged 12 commits intodylanmc:trunkfrom
rileyshahar:fix-clippy
Feb 20, 2023
Merged

Fix clippy and rustfmt errors#3
dylanmc merged 12 commits intodylanmc:trunkfrom
rileyshahar:fix-clippy

Conversation

@rileyshahar
Copy link
Copy Markdown
Collaborator

This is a chore PR to conform the codebase to clippy and rustfmt. I've made
it conformant in as frictionless a way as possible.

  • Format code with rustfmt.
  • Add basic CI.
  • Fix enum variant capitalization.
  • Make library re-exports public.
  • Don't leak private types in APIs. Flags was previously private, but exposed
    in the public DataSource API.
  • Allow module name repetitions for FileDataSource. It's possible we'll want
    to change the name of this object later, but this is the lowest-friction
    change.
  • Clean up FileDataSource::new. It's more idiomatic to use provided methods on
    Option and Result than re-implenting the methods with match or if let.
  • Make doc comments rustdoc-compatible. (Try running cargo doc --open!)
  • Add must_use annotation to constructors. Any non-side-effecting function
    should always have this annotation.
  • Use todo to panic at todo code sites. This is more idiomatic than writing a
    custom panic message.

We deliberately don't run clippy on pedantic mode in CI, because that's
a lot to ask right now.
The `Flags` enum needs to be public, since it's part of the `DataSource`
API.
Possibly we do want a different name for FileDataSource---maybe just
file---but this is the least-friction change.
Using dedicated `Option` and `Result` methods is considered more
readable than reimplementing them by hand.
Try running `cargo doc --open`!
You should always annotate functions with no side effects as `must_use`
to denote that the user has to do something with the returned value.
This is more idiomatic than a custom panic message.
@dylanmc dylanmc merged commit 2d1299b into dylanmc:trunk Feb 20, 2023
@rileyshahar rileyshahar deleted the fix-clippy branch February 20, 2023 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants