Package: guix-patches;
Reported by: Attila Lendvai <attila <at> lendvai.name>
Date: Mon, 24 Jan 2022 12:02:02 UTC
Severity: normal
Done: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
Bug is archived. No further changes may be made.
Message #14 received at 53500 <at> debbugs.gnu.org (full text, mbox):
From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr> To: Attila Lendvai <attila <at> lendvai.name> Cc: 53500 <at> debbugs.gnu.org Subject: Re: [bug#53500] Add 42 rust packages, and update 4 of them Date: Tue, 25 Jan 2022 09:21:14 +0100
Hello,
Attila Lendvai <attila <at> lendvai.name> 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 <attila <at> lendvai.name>
> 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 <attila <at> lendvai.name>
> 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 <attila <at> lendvai.name>
> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.