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

Implement custom Mapping and Value types #8

Merged
merged 8 commits into from
Sep 1, 2023
Merged

Conversation

simu
Copy link
Member

@simu simu commented Aug 31, 2023

This PR introduces custom Mapping and Value types which are compatible with serde_yaml's types, but provide extra state and functionality for handling the Reclass parameters Mapping.

The PR updates the Node parsing implementation to convert the raw parsed parameters into our custom Mapping type, and moves the logic which injects the _reclass_ meta parameter from the raw serde_yaml parameters Mapping to the custom Mapping.

Additionally, we move the as_py_dict() and as_py_obj() methods to the custom Mapping and Value types respectivey, and remove the variants which take serde_yaml types as parameters.

Finally, we update NodeInfo to use the custom Mapping type for parameters.

Checklist

  • The PR has a meaningful title. The title will be used to auto generate the changelog
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency, internal
    as they show up in the changelog

@simu simu added the enhancement New feature or request label Aug 31, 2023
@simu simu force-pushed the feat/custom-parameters-types branch from 1e24aaa to d045b79 Compare August 31, 2023 16:49
simu added 8 commits August 31, 2023 18:56
We introduce our own `Mapping` and `Value` types which are compatible
with `serde_yaml`'s types, but provide extra state and functionality for
handling the Reclass parameters Mapping.
We already introduce `Value::variant()` and `Value::strip_prefix` which we'll
use when implementing Reclass parameter merging.
@simu simu force-pushed the feat/custom-parameters-types branch from d045b79 to 0870274 Compare August 31, 2023 16:56
@simu simu marked this pull request as ready for review September 1, 2023 09:22
@simu simu requested a review from a team September 1, 2023 09:22
Copy link

@bastjan bastjan left a comment

Choose a reason for hiding this comment

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

Looks good, but did not fully go into depth reviewing each (is/as) function in value.rs

@simu simu merged commit fbaf95f into main Sep 1, 2023
@simu simu deleted the feat/custom-parameters-types branch September 1, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants