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

build: relax dependencies #10

Closed

Conversation

QuadnucYard
Copy link

Changes in Cargo.toml

  • Reduce the minimum version requirement of dependencies. memchr can have no default features.
  • (breaking) Move dependencies unrelated to functionality (log, serde_json, tempfile) to dev-dependencies.

Related changes about function only covered in tests are not done yet.

Questions about deps and more changes

I found most macros are not necessary for this serde of yml. Could they be just removed, added feature flags, or moved to another crate?

  • macro_directory, macro_file, macro_get_field: Could be moved to a utility crate, as they depend on filesystem and different serde formats. Besides, the Not Found error message checking in tests can fail where it is not given in English.
  • macro_nested_enum_serde. Just a one-line shorthand.
  • macro_replace_placeholder. It does nothing with yml. Just a string utility.
  • macro_from_number. It may be replaced by a blanket impl:
    impl<T> From<T> for Value
    where
        T: Into<Number>,
    {
        fn from(n: T) -> Self {
            Value::Number(n.into())
        }
    }
    macro_partialeq_numeric does more, but it does not need to be exported either.
  • crate::utilities::directory is only referer in fs related tests. It should belong to a fs utility crate. dep::tempfile and dep:log go with it.
  • serde_yml::modules::path::Path derives Serialize only because a test need it. If this derive is stripped, dep:serde can have no extra feature enabled.
  • anyhow only appears in docs. serde_yml::Result<()> is adequate here.
  • It is surprising that serde_json is a dependency in a yml library.

A radical patch can be found in https://github.com/QuadnucYard/serde_yml/tree/relex-deps-radical. I do not include it here.

btw, could you open the issues channel?

@sebastienrousseau
Copy link
Owner

@QuadnucYard Many thanks for your updates and changes.

I have implemented it in the next release PR, I removed the macros and merged your radical patch.

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