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

Add configuration objects and JSON serialization #231

Closed
wants to merge 2 commits into from

Conversation

miroman9364
Copy link
Contributor

PR Summary

This change adds the configuration data for the File resource. This includes all the initial properties, and code to serialize/deserialize the configuration and the schema.

PR Context

The File configuration will provide the user interface for configuring the File resource behavior.

Added `FileConfiguration` and `HashConfiguration` with serialize/deserialize to JSON. Also added helper to get the schema.

The file resource MVP provides basic configuration of files and their content.
Replace the auto-generated metadata with simpler, fixed text.
The generated metadata for `title` and `description` is coming from the comments.
It's overly verbose and a little out of context.
#[schemars(title = "DSC.FileConfiguration", description = "File resource configuration.")]
pub struct File {
pub path: String,
#[serde(rename = "hash", skip_serializing_if = "Option::is_none")]
Copy link
Member

Choose a reason for hiding this comment

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

Looks like rename is not needed here

Suggested change
#[serde(rename = "hash", skip_serializing_if = "Option::is_none")]
#[serde(skip_serializing_if = "Option::is_none")]

Ok(json) => json,
Err(e) => {
eprintln!("Failed to serialize to JSON: {e}");
String::new()
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to return an empty string if it fails to serialize? Seems better to return a Result<>.

Ok(file) => Some(file),
Err(e) => {
eprintln!("Failed to deserialize from JSON: {e}");
None
Copy link
Member

Choose a reason for hiding this comment

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

Same here, return a Result<>

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