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

✨ zv: support kebab-case in rename_all #899

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

bachp
Copy link

@bachp bachp commented Jul 11, 2024

Also add some tests for case conversion
to ensure renamings are consistent

Copy link
Contributor

@zeenix zeenix left a comment

Choose a reason for hiding this comment

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

LGTM otherwise. Thanks!

zvariant_utils/src/case.rs Outdated Show resolved Hide resolved
zvariant_utils/src/case.rs Outdated Show resolved Hide resolved
Also add some tests for case conversion
to ensure renamings are consistent

Signed-off-by: Pascal Bach <[email protected]>
@bachp bachp force-pushed the zvariant_derive-kebab-case branch from 8ed6e4b to c5e2d63 Compare July 15, 2024 20:58
@bachp bachp requested a review from zeenix July 15, 2024 21:00
Copy link
Contributor

@zeenix zeenix left a comment

Choose a reason for hiding this comment

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

Cool, thanks! It's so much easier to follow now. :) Although, I think we should change the API to only expose snake_case and kebab_case methods in the public API that are thin wrappers around snake_or_kebab_case, which isn't then exposed anymore. Same thing for pascal_or_camel_case. However, this can be done in an separate PR and not necessarily by you. :)

Also for future reference, I'd have split this commit into 3: the new zvariant_utils API, use of it in zbus_macros and finally the new tests.

@zeenix zeenix enabled auto-merge July 16, 2024 11:07
@zeenix zeenix merged commit eeed9a6 into dbus2:main Jul 16, 2024
7 checks passed
@bachp bachp deleted the zvariant_derive-kebab-case branch January 8, 2025 20: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