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

Remove TryInto for Raw and remove Raw config types #63

Open
mgattozzi opened this issue Aug 10, 2021 · 2 comments
Open

Remove TryInto for Raw and remove Raw config types #63

mgattozzi opened this issue Aug 10, 2021 · 2 comments
Labels
good first issue Good for newcomers maintenance Maintenance and gardening

Comments

@mgattozzi
Copy link
Contributor

We have a couple of configs where we first deserialize to a Raw config type and then call try_into to convert to the validated and massaged final config type. This is because we're only using the default Deserialize implementation for config types. We can remove this extra intermediate step and type by either creating a Deserialize implementation ourselves or by using things like deserialize_with for certain fields as needed.

This isn't a high priority item, but would help make the code base a bit easier to work with in some ways. I'm happy to explain in more detail what needs to be done if someone needs more context/guidance in order to understand what to do or if someone does know how to do it then I will review that PR for them. Otherwise I can get around to this at some point in the future if there isn't interest in doing it.

@mgattozzi mgattozzi added good first issue Good for newcomers maintenance Maintenance and gardening labels Aug 10, 2021
@kination
Copy link
Contributor

kination commented Jan 22, 2022

@mgattozzi looks good!
Could you give some example part where Raw type is being used for deserialization?
(I could find in config.rs, but please let me know if there's more)

@kination
Copy link
Contributor

@mgattozzi just want to get sure about implementation.
Currently TomlFastlyConfig is

#[derive(Deserialize)]
struct TomlFastlyConfig {
    local_server: Option<RawLocalServerConfig>,
    ...
    ...
}

and

let local_server = local_server
            .map(TryInto::try_into)
            .transpose()?
            .unwrap_or_default();

so you want this to be changed to

#[derive(Deserialize)]
struct TomlFastlyConfig {
    local_server: Option<String>,
    ...
    ...
}

and make deserialization?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers maintenance Maintenance and gardening
Projects
None yet
Development

No branches or pull requests

2 participants