Skip to content

Conversation

@sjfhsjfh
Copy link
Contributor

@sjfhsjfh sjfhsjfh commented Dec 3, 2025

Summary

  • Replace manual Executable matches with enum_dispatch for Command/Topic.
  • Simplify Self_ subcommand struct.
  • Add enum_dispatch dependency and update Cargo.lock.
  • Fix clippy warnings

Breaking change

  • run_func removed. Use run instead.

Copy link
Collaborator

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I'm not a fan of the enum_dispatch macro. While it makes the code shorter, it also makes it more difficult to understand. Before I could just read the code, now I have to look up and read the docs of the macro first. It's not a trivial expansion, so it takes quite a bit of time to understand. And since it's a proc macro, reading and understanding the macro's implementation is quite a bit of work.

This PR also seems to bundle some unrelated changes, such as the breaking removal of run_func. Such changes should go into separate PRs. However, I'm against removing the run_func at this point because it is a breaking change and keeping it around for a bit longer does no harm.

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