[bug#70317] [PATCH 00/13] Add soju

[Sharlatan Hellseher <sharlatanus@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3604 bytes --]


Hi Jesse,

Thank you for the patches.

From the first review round it looks like QA is not happy with some git
conflicts while applying the patches.

I've checked the general styling and some of the packages have lables,
some not, some use list style some not, let's still to one style across
all of the. WDYT?

Some review point for v2:
- rebase to master, fix git conflict

- do not use input labels e.g.
from
--8<---------------cut here---------------start------------->8---
+    (inputs `(("linux-pam" ,linux-pam)))
--8<---------------cut here---------------end--------------->8---
to
--8<---------------cut here---------------start------------->8---
+    (inputs (list linux-pam))
--8<---------------cut here---------------end--------------->8---

- use list style everywhere in arguments e.g.
from
--8<---------------cut here---------------start------------->8---
+ (arguments
+     `(#:go ,go-1.18
+       #:import-path "git.sr.ht/~emersion/go-sqlite3-fts5"
--8<---------------cut here---------------end--------------->8---
to
--8<---------------cut here---------------start------------->8---
+ (arguments
+     (list
+       #:go ,go-1.18
+       #:import-path "git.sr.ht/~emersion/go-sqlite3-fts5"
--8<---------------cut here---------------end--------------->8---

- check import-path, it should be the same as may see in go.mod
from 
--8<---------------cut here---------------start------------->8---
+      #:import-path "github.com/golang-jwt/jwt"))))
--8<---------------cut here---------------end--------------->8---
to
--8<---------------cut here---------------start------------->8---
+      #:import-path "github.com/golang-jwt/jwt/v4"))))
--8<---------------cut here---------------end--------------->8---
https://github.com/golang-jwt/jwt/blob/v4.5.0/go.mod
If the module name does not contain "v<version>" try to update existing
package in Guix.

- description of the package needs to be more informative and longer
  than synopsis
--8<---------------cut here---------------start------------->8---
+    (synopsis "Standalone FTS5 extension for go-sqlite3")
+    (description "Standalone FTS5 extension for
+@@url{https://github.com/mattn/go-sqlite3,go-sqlite3}.")
--8<---------------cut here---------------end--------------->8---

- explain why tests are disabled
--8<---------------cut here---------------start------------->8---
+    (arguments
+     (list
+      #:tests? #f
+      #:import-path "gopkg.in/irc.v4"
+      #:unpack-path "gopkg.in/irc.v4"))
--8<---------------cut here---------------end--------------->8---

- according to the description, soju may be placed in (gnu packages irc)
--8<---------------cut here---------------start------------->8---
+    (synopsis "User-friendly IRC bouncer") ;
--8<---------------cut here---------------end--------------->8---

- try to apply G-expression which may simplify the package a lot (se
  examples in golang-* modules)
--8<---------------cut here---------------start------------->8---
+       #:phases (modify-phases %standard-phases
+                  (replace 'build
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (with-directory-excursion "src/git.sr.ht/~emersion/soju"
+                        (setenv "SYSCONFDIR"
+                                (string-append (assoc-ref outputs "out")
+                                               "/etc"))
+                        (invoke "make"))))
--8<---------------cut here---------------end--------------->8---


Looking forward for v2!

--
Oleg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]