Subject: Re: [bug#53500] Add 42 rust packages, and update 4 of them

From: Nicolas Goaziou <[email-masked]>
Date: Tue, 25 Jan 2022 09:21:14 +0100

Hello,

Attila Lendvai <[email-masked]> writes:

> please find the updated patch attached. it still has a few holes where
> i had no idea what to write...

Thanks. Some more comments follow.

> From 76c538bb3331b904790a827d7b762e057052de0c Mon Sep 17 00:00:00 2001
> From: Attila Lendvai <[email-masked]>
> Date: Wed, 19 Jan 2022 09:29:13 +0100
> Subject: [PATCH 02/46] gnu: Add rust-parity-bytes 0.1.2.

In commit messages, you should mention the name of the variable you
added, not the name of the package and its version. So, the above should
be:

  gnu: Add rust-parity-bytes.
>
> * gnu/packages/crates-io.scm (rust-parity-bytes): New variable.

BTW, the name of the variable is wrong. It should be
`rust-parity-bytes-0.1'.

> +(define-public rust-parity-bytes

See above.

> +  (package
> +    (name "rust-parity-bytes")
> +    (version "0.1.2")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (crate-uri "parity-bytes" version))
> +       (file-name (string-append name "-" version ".tar.gz"))
> +       (sha256
> +        (base32 "0rww406dih6yxr4iadz7l07sibklqywxz10gjzdqn4r04hx6xd8n"))))
> +    (build-system cargo-build-system)
> +    (arguments `(#:skip-build? #t))
> +    (home-page "https://github.com/paritytech/parity-common")
> +    (synopsis "General bytes-related utilities")
> +    (description synopsis)

As discussed on IRC, the construct above is to avoid. I suggest:

  "This package provides general bytes-related utilities, including
  a pretty-printer."

> Subject: [PATCH 03/46] gnu: Add rust-ethereum-types 0.9.2 and 0.12.1.

See above for the remark about commit messages.
>
> * gnu/packages/crates-io.scm (rust-ethereum-types-0.9): New variable.
> (rust-ethereum-types-0.12):  New variable.
> ---
>  gnu/packages/crates-io.scm | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm
> index fb07701e16..0013ae28a2 100644
> --- a/gnu/packages/crates-io.scm
> +++ b/gnu/packages/crates-io.scm
> @@ -20081,6 +20081,42 @@ (define-public rust-escargot-0.3
>         (("rust-serde" ,rust-serde-1)
>          ("rust-serde-json" ,rust-serde-json-1))))))
>  
> +(define-public rust-ethereum-types-0.12
> +  (package
> +    (name "rust-ethereum-types")
> +    (version "0.12.1")
> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri (crate-uri "ethereum-types" version))
> +        (file-name
> +         (string-append name "-" version ".tar.gz"))
> +        (sha256
> +         (base32
> +          "1bxxacsmb9majw7vd4sndv4dhw3g9srhf7flwq39yy7yaxq6y4q5"))))
> +    (build-system cargo-build-system)
> +    (arguments
> +     `(#:skip-build? #t))
> +    (home-page "https://github.com/paritytech/parity-common")
> +    (synopsis "Crate used in Parity Technologies projects")
> +    (description synopsis)

See above. I don't have anything meaningful to suggest, but the bare
minimum is

  (description "This is crate an internal crate used in Parity
  Technologies projects.")

BTW, should it be public?

> Subject: [PATCH 05/46] gnu: Add rust-base64ct 1.0.1.
>
> * gnu/packages/crates-io.scm (rust-base64ct-1.0.1): New variable.

Do you need this variable? We already provide 1.1.1 version. Could
requirements be relaxed instead? See, e.g., rust-average-0.13.
> Subject: [PATCH 09/46] gnu: Add rust-scrypt 0.5.0.
>
> * gnu/packages/crates-io.scm (rust-scrypt-0.5): New variable.

Usually, we also add:

  (rust-scrypt-0.3): Inherit from above.
> +    (home-page "https://serde.rs")
> +    (synopsis "AST representation used by Serde derive macros")
> +    (description "AST representation used by Serde derive macros.
> Unstable.")

Please fix the description.

> +    (synopsis "Bare essentials of @code{std::io} for use in @code{no_std}")
> +    (description
> +      "The bare essentials of @code{std::io} for use in @code{no_std}.  Alloc
> +support is optional.")

Please fix the description. This is not a full sentence.

> From ad11a18a8c4468c9895d431962a5a76dd29894de Mon Sep 17 00:00:00 2001
> From: Attila Lendvai <[email-masked]>
> Date: Wed, 19 Jan 2022 15:06:00 +0100
> Subject: [PATCH 13/46] gnu: Add rust-darling 0.13.1.

See my comment above about mentionning inheritance.

> +    (synopsis "Helper crate for @code{rust-darling}")
> +    (description
> +     "Internal support for @code{rust-darling}, a proc-macro library for
> +reading attributes into structs when implementing custom derives.")
> +    (license license:expat)))

Please turn the description into a full sentence.
> +    (synopsis "A library for creating compact sets of enums")

Synopses should not start with an article. Please remove it.

> +    (description
> +      "This package provides a rust library for creating compact sets of enums.")
> +    (license (list license:expat license:asl2.0))))

rust -> Rust.

> +    (synopsis "An internal helper crate for enumset")

See above.

> +    (description "An internal helper crate for enumset.  Not public
> API.")

Please turn it into full sentences.

> +    (synopsis "Macros for #[derive(JsonSchema)], for use with
> schemars")

I suggest to wrap @code{...} around #[derive...]

> +    (description synopsis)

Please write a description, even if it is:

  This package provides macros for ...

> From b42eeb02d631ce64152a8244c1b56aba51b0b66e Mon Sep 17 00:00:00 2001
> From: Attila Lendvai <[email-masked]>
> Date: Wed, 19 Jan 2022 15:59:23 +0100
> Subject: [PATCH 19/46] gnu: rust-zeroize: Update to 1.5.0.
>
> Also update rust-zeroize-derive to 1.3.1.

The commit message is not formatted properly:

  * gnu/packages/crates-io.scm (rust-zeroize): Update to 1.5.0.
  (rust-zeroize-derive): Update to 1.3.1.
>
> * gnu/packages/crates-io.scm (rust-zeroize-1.4): New variable.
> * gnu/packages/rust-apps.scm (rbw): Stick to the v1.4 line of rust-zeroize.

Can you relax requirements instead? Adding an intermediate crate is
a last-resort solution.

> Also reword the description.

[description]: Reword it.

> +    (synopsis
> +      "Rust wrapper library for Pieter Wuille's `libsecp256k1`. Implements ECDSA and BIP 340 signatures for the SECG elliptic curve group secp256k1 and related utilities.")

Synopsis should be shortened.

> +    (description
> +      "Rust wrapper library for Pieter Wuille's `libsecp256k1`.  Implements ECDSA and
> +BIP 340 signatures for the SECG elliptic curve group secp256k1 and related
> +utilities.")

Full sentences, please.

> +    (synopsis "Macros to define custom fixed-size hash types")
> +    (description "Rust macros to define custom fixed-size hash types.")
> +    (license (list license:expat license:asl2.0))))

See above.

I won't comment further because further remarks are of the same type.

Could you send an updated patch set?

Thank you for your work.

Regards,
-- 
Nicolas Goaziou