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

chore: allow empty mapping string #33

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Conversation

lukebond
Copy link
Contributor

it should be possible to specify an empty string as a mapping, e.g. let's say you want to copy some subtree of an object into the root of another, or vise versa. the to and from mapping fields are both required so we must support an empty string to indicate this case. as the code was before it would become the jsonpath $. because we were simply checking if the option was Some(String).

@lukebond lukebond requested a review from mkmik July 10, 2023 16:22
@lukebond
Copy link
Contributor Author

lukebond commented Jul 10, 2023

i'm not happy with the Rust here. rust playground says this should work, but it doesn't: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021 so we have the version i've put in the PR. it works.

update: ah, it works in the playground because in that example it's a &str not a String. i am rusty indeed. any improvements welcome or we can merge like this.

@lukebond
Copy link
Contributor Author

an alternative solution is to make those fields options in the crd, so the user just leaves one or the other out to specify the root. unsure if that's more or less intuitive

@lukebond lukebond merged commit d4caff2 into main Jul 11, 2023
@lukebond lukebond deleted the fix/allow-empty-mapping branch July 11, 2023 09:29
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