Subject: Re: [bug#53376] [PATCH] gnu: Add fan2go

From: Maxime Devos <[email-masked]>
Date: Thu, 20 Jan 2022 11:03:01 +0100

Hi,

I took a quick look at the synopsi and descriptions:

Elais Player schreef op wo 19-01-2022 om 15:16 [-0700]:
> + (define-public go-github-com-atomicgo-cursor
> +  (package
> +    [...]
> +    (synopsis "Description")

A synopsis is missing.

> +    (description "Package cursor contains cross-platform methods to
> move the
> +terminal cursor in different directions. 

This doesn't seem grammatical, how about ‘This package contains [...]’?

>  This package can be used to create
> +interactive CLI tools and games, live charts, algorithm
> visualizations and
> +other updatable output of any kind.")
> +    (license license:expat)))
> +
> +(define-public go-github-com-gookit-color
> +  [...]
> +    (home-page "https://github.com/gookit/color")
> +    (synopsis "Command line color library")

Is it a command line tool, or a library?  It cannot be both
(with some exceptions, e.g. glibc can be run). 

How about: ‘Library for setting colors in the terminal’. 

> +    (description "Package color is Command line color library.

Likewise (grammar), and why is ‘Command’ capitalised?
How about: ‘This package allows setting the colours used in the
terminal ...’

>   Support rich
> +color rendering output, universal API method, compatible with
> Windows system

Likewise (grammar), the imperative doesn't make much sense here

> +")
> +    (license license:expat)))
> +
> +(define-public go-github-com-pterm-pterm
> +  (package
> +    ]...]
> +    (synopsis "Console output beautifier")
> +    (description "Pterm is a modern go module to beautify console
> output.  It
> +can be used without configuration, but if desired, everything can be
> +customized down to the smallest detail.")

‘Modern, smallest detail’: this is getting a bit close to marketing
phrases (see (guix)Synopses and Descriptions), although it is rather
mild here.

> +    (license license:expat)))
> +
> +(define-public go-github-com-marvinjwendt-testza
> +  (package
> +    [...]
> +    (synopsis "Testing framework for go")
> +    (description "Package testza is a full-featured testing
> framework for Go.

In the synopsis 'go' is not capitalised, while in the description, it
is.  There is some inconsistency here.

> +It integrates with the default test runner, so you can use it with
> the
> +standard `go test` tool.

Guix doesn't use markdown, it uses Texinfo, so IIUC this needs to be
@command{go test}.  Correct markup helps with

>   Testza contains easy to use methods, like
> +assertions, output capturing, mocking, and much more.")
> +    (license license:expat)))
> +
> +(define-public go-github-com-xo-terminfo
> +  (package
> +    [...]
> +    (synopsis "Terminfo file reader in pure go")
> +    (description "Package terminfo provides a pure-Go implementation
> of
> +reading information from the terminfo database.")

Go is capitalised inconsistently.  Also, IIUC the proposition needs to
be 'for' and not 'of'.

> +    (license license:expat)))
> +
> +(define-public go-github-com-klauspost-cpuid-v2
> +  (package
> +    [...]
> +    (synopsis "CPU feature identification for Go")
> +    (description "Cpuid provides information about the CPU running
> the current
> +program.  CPU features are detected on startup, and kept for fast

I think ‘kept’ -> ‘remembered’ would be clearer here (YMMV).

> access
> +through the life of the application.  Currently x86 / x64
> (AMD64/i386) and ARM
> +(ARM64) is supported, and no external C (cgo) code is used, which
> should make
> +the library very easy to use.")

AFAIK external C code works just fine in Guix without any complications
so this remark doesn't seem necessary in Guix.  But then I never
actually write any Go code, so take it with some grain of salt.

> +    (license license:expat)))

Since not all architectures are supported, I recommend setting
'supported-systems'.

> +[...]
> +
> +(define-public go-github-com-guptarohit-asciigraph
> +  (package
> +    (name "go-github-com-guptarohit-asciigraph")
> +    (version "0.5.2")
> +    (source
> +      (origin
> +        (method git-fetch)
> +        (uri (git-reference
> +               (url "https://github.com/guptarohit/asciigraph")
> +               (commit (string-append "v" version))))
> +        (file-name (git-file-name name version))
> +        (sha256
> +          (base32
> "1hkxaqs30hbybbxapr8vng09r6wr8afpkqkn0g6aj1vl3fvhjn49"))))
> +    (build-system go-build-system)
> +    (arguments '(#:import-path "github.com/guptarohit/asciigraph"))
> +    (home-page "https://github.com/guptarohit/asciigraph")
> +    (synopsis "Package to make a lightweight ASCII line graph")
> +    (description "Go package to make a lightweight ASCII line graph
> for CLI
> +apps.")
> +    (license license:bsd-3)))
> +
> +(define-public go-github-com-oklog-run
> +  (package
> +    (name "go-github-com-oklog-run")
> +    (version "1.1.0")
> +    (source
> +      (origin
> +        (method git-fetch)
> +        (uri (git-reference
> +               (url "https://github.com/oklog/run")
> +               (commit (string-append "v" version))))
> +        (file-name (git-file-name name version))
> +        (sha256
> +          (base32
> "0r55p3kgdkgw55i33lqvvvl60mjp92mhd1170m980sw98z9150jk"))))
> +    (build-system go-build-system)
> +    (arguments '(#:import-path "github.com/oklog/run"))
> +    (home-page "https://github.com/oklog/run")
> +    (synopsis "Universal mechanism to manage goroutine lifecycles")
> +    (description "Package run implements an actor-runner with

‘Package run’ -> ‘This package’?
> deterministic
> +teardown.  It is somewhat similar to package errgroup, except it

‘package errgroup’ -> @code{errgroup}?

> does not
> +require actor goroutines to understand context semantics.  This
> makes it
> +suitable for use in more circumstances; for example, goroutines
> which are
> +handling connections from net.Listeners, or scanning input from a
> closable
> +io.Reader.")

Texinfo markup (@code{...}) for io.Reader and net.Listeners.

> +    (license license:asl2.0)))
> +
> +(define-public go-github-com-tomlazar-table
> +  (package
> +    (name "go-github-com-tomlazar-table")
> +    (version "0.1.0")
> +    (source
> +      (origin
> +        (method git-fetch)
> +        (uri (git-reference
> +               (url "https://github.com/tomlazar/table")
> +               (commit (string-append "v" version))))
> +        (file-name (git-file-name name version))
> +        (sha256
> +          (base32
> "017ps5n44xvdl2b656v89qi0gib95fz60vqvnpq7kjzjn9sr3awy"))))
> +    (build-system go-build-system)
> +    (arguments '(#:import-path "github.com/tomlazar/table"))
> +    (inputs
> +     `(("go-github-com-stretchr-testify" ,go-github-com-stretchr-
> testify)))
> +    (propagated-inputs
> +      `(("go-github-com-mgutz-ansi" ,go-github-com-mgutz-ansi)
> +        ("go-github-com-mattn-go-isatty" ,go-github-com-mattn-go-
> isatty)
> +        ("go-github-com-mattn-go-colorable"
> +         ,go-github-com-mattn-go-colorable)))
> +    (home-page "https://github.com/tomlazar/table")
> +    (synopsis "Colorful tables in go with less effort")
> +    (description "Print tabular data on the command line using ansi
> color
> +esacape codes.

The imperative doesn't make much sense here.  Also, a typo: ‘esacapa’.
And 'ansi' probably needs to be capitalised to ‘ANSI’

>   Support for writing the ouput based on the fields in a struct

Likewise (imperative).

> +and for defining and creating the table manully using the underlying
> object.")
> +    (license license:expat)))
> +
> +(define-public go-github-com-looplab-tarjan
> +  (package
> +    [..]
> +    (synopsis "Graph loop detection using Tarjan's algorithm")
> +    (description "Tarjan is a graph loop detection function using
> Tarjan's
> +algorithm.  The algorithm takes a input graph and produces a slice

IIUC, the standard graph theory terminology would be ‘... and produces
a list of the strongly connected components’.  AFAIK ‘slice’ is not
used in gaph theory.

> where each
> +item is a slice of strongly connected vertices.  The input graph is
> in form of
> +a map where the key is a graph vertex and the value is the edges in
> for of a
> +slice of vertices.")

Aside from that,, it's a nice description.

> +    (license license:asl2.0)))
> +
> +(define-public go-github-com-md14454-gosensors
> +  (package
> +    (name "go-github-com-md14454-gosensors")
> +    (version "0.0.0-20180726083412-bded752ab001")
> +    (source
> +      (origin
> +        (method git-fetch)
> +        (uri (git-reference
> +               (url "https://github.com/md14454/gosensors")
> +               (commit (go-version->git-ref version))))
> +        (file-name (git-file-name name version))
> +        (sha256
> +          (base32
> "08ffz56h93rcsyp84zxirdj3azb9rkql7sc0dj22pp17w11qlwhx"))))
> +    (build-system go-build-system)
> +    (propagated-inputs
> +     `(("lm-sensors:lib" ,lm-sensors "lib")))

Is propagation necessary, or would 'inputs' suffice?
Propagation can cause profile collisions and in some cases make profile
building slow, and hence is to be avoided.

> +    (arguments '(#:import-path "github.com/md14454/gosensors"
> +                 #:tests? #f))

Why are tests disabled?  If tests fail, that can mean the tests
function as intended because they might be detecting a real issue.

> +    (home-page "https://github.com/md14454/gosensors")
> +    (synopsis "Go bindings for libsensors")
> +    (description "Go bindings for libsensors.so from the lm-sensors

The .so seems to be an implementation detail,
‘Go bindings for @code{lm-sensors}’?

> project
> +via external C bindings.")
> +    (license license:expat)))
> +[...]
> +(define-public fan2go
> +  (package
> +    (name "fan2go")
> +    [...]
> +    (arguments
> +     '(#:import-path "github.com/markusressel/fan2go"
> +       #:tests? #f))

Why are tests disabled here?
This is a command line utility, not a Go library, so propagation
is probably unnecessary (though some 'wrap-program' might be required).

> +    [...]
> +    (home-page "https://github.com/markusressel/fan2go")
> +    (synopsis "Fan speed control daemon based on reading temperature
> sensors")

Something like
‘Automated fan speed control daemon preventing overheating’
(assuming that's correct) seems more descriptive.  While reading
temperature sensors seems unavoidable in an implementation, it still
seems an implementation detail to me.

> +    (description "fan2go is a simple fan control daemon that relies

@command{fan2go}

> on
> +lm-sensors to get both temperature and RPM sensor readings, as well

@code{fan2go}

> as PWM

What's RPM and PWM?  From (guix)Synopses and descriptions:

‘[...] avoid using acronyms without first introducing them.’

> +controls, so you will have to set it up first.")

What setup is needed here?  Can this requirement be eliminated in Guix?

Greetings,
Maxime.