unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Sarah Morgensen <iskarian@mgsn.dev>
To: phodina <phodina@protonmail.com>
Cc: 49578@debbugs.gnu.org
Subject: [bug#49578] [PATCH v3 1/2] gnu: Add bolt.
Date: Wed, 22 Sep 2021 18:11:23 -0700	[thread overview]
Message-ID: <86mto4w0xw.fsf@mgsn.dev> (raw)
In-Reply-To: <IpWOs4rfABoT7fYcMUB85zS3FRn8ZwpMv_PQzYRjT6aj14ZdfbW8KLgQB9oOJFHKni5PmV5CSNs4h9qbDCpG3DgqGF2btkjbIRCabQV24Zo=@protonmail.com> (phodina@protonmail.com's message of "Sat, 18 Sep 2021 11:20:38 +0000 (4 days, 12 hours, 42 minutes ago)")

Hello Petr,

Thanks for the updated patch :)

I don't have any experiences with services, so I can't comment on the
second patch in this series, and I don't have a Thunderbolt controller
or devices, so I can't test the actual program, but other than the
commit message/synopsis/description this package LGTM.

phodina <phodina@protonmail.com> writes:

> * gnu/packages/linux.scm: (bolt): New variable.
                          ^ no ":"

> +    (synopsis "Userspace system daemon for Thunderbolt")

Consider this instead (taken from their repository tagline, more
understandable to more users):

       (synopsis "Thunderbolt 3 device manager")

> +    (description "Userspace daemon @code{boltd} exposes devices via D-Bus to clients.
> +It stores database of previously authorized devices and depending
> +on the policy set for the individual devices, automatically authorize newly
> +connected devices without user interaction.  It also adapts its behaivour when
> +iommu support is detected.
> +Command line utility 'boltctl' can be used to manage thundebolt devices.
> +It can list devices, monitor changes and initiate authorization of device.")

Consider this instead:

    (description "This package provides @command{boltd}, a userspace daemon
for Thunderbolt devices, and @command{boltctl}, a command-line utility for
managing those devices.

The daemon @command{boltd} exposes devices via D-Bus to clients.  It also
stores a database of previously authorized devices and will, depending on the
policy set for the individual devices, automatically authorize newly connected
devices without user interaction.

The command-line utility @command{boltctl} manages Thunderbolt devices via
@command{boltd}.  It can list devices, monitor changes, and initiate
authorization of devices.")


> +    (home-page "https://gitlab.freedesktop.org/bolt/bolt")
> +    (license license:gpl2+)))
> +
>  (define-public jitterentropy-rngd
>    (package
>      (name "jitterentropy-rngd")
> --
> 2.32.0

With those changes, or similar:

* Package review checklist (WIP)
Lack of a check is not proof of noncompliance.

1. [X] Package builds (guix build package)
     [X] x86_64  [ ] aarch64  [ ] armhf     [ ] powerpc64le
     [ ] i686    [ ] i586     [ ] mips64le
2. [X] Build is reproducible (guix build --rounds=n package)
3. [X] Tests enabled (if available) or disabled with in-source comment
4. [X] No extraneous dependencies (guix size package)
5. [X] No unnecessary pre-built binaries/blobs
6. [X] Dependencies unvendored when available in Guix
7. [X] Cross-compile friendly (cc-for-target, patches with inputs)
8. [X] License matches source
9. [X] No problematic lints (guix lint package)
10. [X] Formatting follows guidelines
    - [X] Lines wrap at 80 characters
    - [X] Indentation passes etc/indent-code.el
    - [X] Two spaces used between sentences

For new packages:
1. [X] Source url is robust
   * mirror:// used with url-fetch when possible
   * No auto-generated archives from source repo (prefer cloning)
2. [X] Synopsis is short, descriptive, and meaningful to a wide audience
3. [X] Description is objective, takes 5-10 lines, uses full sentences,
   and provides the information users need to decide whether the
   software fits their needs.

There are lints for CVEs, but they are for the Bolt CMS, not this
project.

--
Sarah




  reply	other threads:[~2021-09-23  1:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 16:46 [bug#49578] [PATCH] Add bolt phodina via Guix-patches via
2021-07-22  1:50 ` Sarah Morgensen
2021-07-22 11:32   ` phodina via Guix-patches via
2021-07-22 11:36     ` phodina via Guix-patches via
2021-07-22 20:02       ` Sarah Morgensen
2021-09-18 11:20 ` [bug#49578] [PATCH v3 1/2] gnu: " phodina via Guix-patches via
2021-09-23  1:11   ` Sarah Morgensen [this message]
2021-11-05  8:39     ` phodina via Guix-patches via
2022-08-14 22:00       ` [bug#49578] [PATCH v4] " phodina via Guix-patches via
2022-12-14 11:22         ` [bug#49578] [PATCH] " Ludovic Courtès
2023-01-05 21:28         ` Ludovic Courtès
2023-01-10  2:49           ` phodina via Guix-patches via
2021-09-18 11:21 ` [bug#49578] [PATCH v3 2/2] services: Add a service for bolt phodina via Guix-patches via
2022-01-22  6:55 ` [bug#49578] [PATCH] Add bolt David Conner
2022-02-02 16:19   ` David Conner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86mto4w0xw.fsf@mgsn.dev \
    --to=iskarian@mgsn.dev \
    --cc=49578@debbugs.gnu.org \
    --cc=phodina@protonmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).