Skip to content

refactor!: clippy and idiomatic Rust compliance#62

Merged
willstott101 merged 4 commits intomainfrom
refactor/clippy-fixes
Aug 21, 2025
Merged

refactor!: clippy and idiomatic Rust compliance#62
willstott101 merged 4 commits intomainfrom
refactor/clippy-fixes

Conversation

@roderickvd
Copy link
Member

  • Add clippy and Rust idiom lint attributes
  • Refactor code for clippy::pedantic and idiomatic style
  • Replace unwrap() with infallible constructors where possible
  • Improve error handling and documentation
  • Use assert! instead of panic! for invariants
  • Prefer From over Into for conversions
  • Update examples and tests for new API
  • Remove unused imports and code
  • Add Default implementation for Responder

- Add clippy and Rust idiom lint attributes
- Refactor code for clippy::pedantic and idiomatic style
- Replace unwrap() with infallible constructors where possible
- Improve error handling and documentation
- Use assert! instead of panic! for invariants
- Prefer From over Into for conversions
- Update examples and tests for new API
- Remove unused imports and code
- Add Default implementation for Responder
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@willstott101
Copy link
Contributor

willstott101 commented Aug 21, 2025

Thanks for picking up some of this lib. I have certainly been putting it off a bit.

I generally have avoided too much formatting and lint fixing within dns_parser under the idea that we'd be able to track some updates from upstream. But that seems to just not be practical (#56 showed me how far they've diverged).

Anyway this generally LGTM. If you're fixing the code according to some static analysis tool though I'd request we run that tool in the CI.

@willstott101
Copy link
Contributor

willstott101 commented Aug 21, 2025

Not sure what toml formatter you're using (tried taplo which used 2 spaces not 4 https://github.com/eclipse-zenoh/zenoh/pull/2057/files ) - it'd be nice to get that aligned too. Anyway merging as is.

@willstott101 willstott101 merged commit 2c9f899 into main Aug 21, 2025
4 checks passed
@roderickvd
Copy link
Member Author

Thanks for picking up some of this lib. I have certainly been putting it off a bit.

No problem, glad to see you around. Though I got write access here from librespot-org, I don't want to mess too much with your crate with our your say so.

Anyway this generally LGTM. If you're fixing the code according to some static analysis tool though I'd request we run that tool in the CI.

Thanks! This is with all clippy lints, including the pedantic ones. We can enable that in lib.rs or Cargo.toml if you want?

Not sure what toml formatter you're using (tried taplo which used 2 spaces not 4 https://github.com/eclipse-zenoh/zenoh/pull/2057/files ) - it'd be nice to get that aligned too. Anyway merging as is.

I'm using Zed's toml extension.

@roderickvd
Copy link
Member Author

This of course caused the open PRs to have merge conflicts. I can help you resolve them, if you'd like.

@willstott101
Copy link
Contributor

You're welcome to write to the repo if you like. I'll make a CONTRIBUTING.md in a bit if it helps to avoid us stepping on each other. I pushed to your branch adding CI for clippy before merging. Zed says it uses taplo so IDK where the indentation difference comes from :/ https://zed.dev/docs/languages/toml

I have started rebasing/merging on #64 will take a closer look at the others next

@willstott101
Copy link
Contributor

Yanked and re-released as 0.10.1

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.

3 participants