Skip to content
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

Breaking change in zenoh_macros::kedefine API #778

Closed
2 tasks done
fuzzypixelz opened this issue Mar 1, 2024 · 2 comments · Fixed by #783
Closed
2 tasks done

Breaking change in zenoh_macros::kedefine API #778

fuzzypixelz opened this issue Mar 1, 2024 · 2 comments · Fixed by #783
Labels
bug Something isn't working

Comments

@fuzzypixelz
Copy link
Member

fuzzypixelz commented Mar 1, 2024

Describe the bug

As of #752, the zenoh_macros::kedefine macro generates getters that return &zenoh_keyexpr::keyexpr for key expression format specs other than ** instead of returning Option<&zenoh_keyexpr::keyexpr> which was the behavior before the pull request in question was merged.

This is a breaking change for users of key expression format specs other than **, e.g. zenoh-plugin-ros1.

@p-avital

To reproduce

  1. Sync zenoh-plugin-ros1's lockfile with Zenoh

System info

@fuzzypixelz fuzzypixelz added the bug Something isn't working label Mar 1, 2024
@fuzzypixelz
Copy link
Member Author

Otherwise if this change is intentional, the documentation should be updated to reflect it:

Parsed will have a method named after each spec's id that returns Option<&keyexpr>. That Option will only be None if the spec's format was ** and matched a sequence of 0 chunks.

@p-avital
Copy link
Contributor

p-avital commented Mar 4, 2024

I was unsatisfied with returning Options that would always be Some, so I opted for tanking the break. The generated docs for each individual method was updated, #783 updates the global docs :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants