diff mbox series

[bug#72452] add guile-swayer package

Message ID 2c614792845a675a1296bcbeb6ef8d8502d51aba.1722717063.git.mrh57@posteo.net
State New
Headers show
Series [bug#72452] add guile-swayer package | expand

Commit Message

Alec Barreto Aug. 3, 2024, 8:31 p.m. UTC
guile-swayer provides guile bindings to manipulate windows via the sway wayland compositor

Change-Id: If579694d8fb20bec5f3fd542430783a25a6c155b
---
 gnu/packages/guile-xyz.scm | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)


base-commit: b20956651a53a8f23828fdeb6945e1a31e6997a8

Comments

Nicolas Graves Sept. 7, 2024, 4:15 p.m. UTC | #1
Hi mrh,

A few remarks below.

On 2024-08-03 20:31, mrh wrote:

> guile-swayer provides guile bindings to manipulate windows via the sway wayland compositor
>
> Change-Id: If579694d8fb20bec5f3fd542430783a25a6c155b
> ---
>  gnu/packages/guile-xyz.scm | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm
> index 3ce4f6a8d6..5693bbde0f 100644
> --- a/gnu/packages/guile-xyz.scm
> +++ b/gnu/packages/guile-xyz.scm
> @@ -1027,6 +1027,24 @@ (define-public guile2.2-filesystem
>      (name "guile2.2-filesystem")
>      (inputs (list guile-2.2))))
>  
> +(define-public guile-swayer
> +  (package
> +    (name "guile-swayer")
> +    (version "0.2.0")
> +    (home-page "https://github.com/ebeem/guile-swayer")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/ebeem/guile-swayer")
> +             (commit "9c962281f405453fb3770dd0546ef6951c9236dd")))
> +       (sha256 (base32 "09c0143q9sm75xp1qz7a7ihdqfwqg4w8nlq0mmnivhvamww775ss"))))

This line exceeds 79 characters, you should put base32 part on the next line.

> +    (native-inputs (list guile-3.0))
> +    (build-system guile-build-system)
> +    (synopsis "Extensible Guile bindings for SwayWM")
> +    (description "Extensible Guile bindings for SwayWM")

The description needs to be more informative. What about

"This package provides extensible Guile bindings for the Sway window
manager.  It can be used to query Sway, assign keybindings and listen to
events in Guile."

> +    (license license:expat)))
> +

Otherwise the package builds properly, I'll be happy to review that once
you send a v2.

Please also abstain from opening duplicate issues, thanks!
Alec Barreto Sept. 7, 2024, 6:17 p.m. UTC | #2
Hi Nicolas,

Thanks for the review! I have fixed the issues in the forthcoming v2 
patch.

Best,
Alec

On 2024-09-07 12:15 pm, Nicolas Graves wrote:
> Hi mrh,
> 
> A few remarks below.
> 
> On 2024-08-03 20:31, mrh wrote:
> 
>> guile-swayer provides guile bindings to manipulate windows via the 
>> sway wayland compositor
>> 
>> Change-Id: If579694d8fb20bec5f3fd542430783a25a6c155b
>> ---
>>  gnu/packages/guile-xyz.scm | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm
>> index 3ce4f6a8d6..5693bbde0f 100644
>> --- a/gnu/packages/guile-xyz.scm
>> +++ b/gnu/packages/guile-xyz.scm
>> @@ -1027,6 +1027,24 @@ (define-public guile2.2-filesystem
>>      (name "guile2.2-filesystem")
>>      (inputs (list guile-2.2))))
>> 
>> +(define-public guile-swayer
>> +  (package
>> +    (name "guile-swayer")
>> +    (version "0.2.0")
>> +    (home-page "https://github.com/ebeem/guile-swayer")
>> +    (source
>> +     (origin
>> +       (method git-fetch)
>> +       (uri (git-reference
>> +             (url "https://github.com/ebeem/guile-swayer")
>> +             (commit "9c962281f405453fb3770dd0546ef6951c9236dd")))
>> +       (sha256 (base32 
>> "09c0143q9sm75xp1qz7a7ihdqfwqg4w8nlq0mmnivhvamww775ss"))))
> 
> This line exceeds 79 characters, you should put base32 part on the next 
> line.
> 
>> +    (native-inputs (list guile-3.0))
>> +    (build-system guile-build-system)
>> +    (synopsis "Extensible Guile bindings for SwayWM")
>> +    (description "Extensible Guile bindings for SwayWM")
> 
> The description needs to be more informative. What about
> 
> "This package provides extensible Guile bindings for the Sway window
> manager.  It can be used to query Sway, assign keybindings and listen 
> to
> events in Guile."
> 
>> +    (license license:expat)))
>> +
> 
> Otherwise the package builds properly, I'll be happy to review that 
> once
> you send a v2.
> 
> Please also abstain from opening duplicate issues, thanks!
Nicolas Graves Sept. 9, 2024, 4:46 a.m. UTC | #3
Some other nitpicks:

1) (commit field should rather refer to version such as
 (commit (string-append "v" version))

This makes it easier to update.

2) guix lint complains about
guile-swayer@0.2.0: the source file name should contain the package name

You're missing
       (file-name (git-file-name name version))
in the origin. This makes it easier to find things in the store.
Alec Barreto Sept. 9, 2024, 4:21 p.m. UTC | #4
Thanks again for the feedback.

> 1) (commit field should rather refer to version such as
>  (commit (string-append "v" version))

I made this change, but as =(commit version)= since there is no "v" in 
the release titles.

I think =guix lint= and =guix style= should now be happy :)
Nicolas Graves Sept. 10, 2024, 8:14 a.m. UTC | #5
On 2024-09-09 16:21, mrh57@posteo.net wrote:

> Thanks again for the feedback.
>
>> 1) (commit field should rather refer to version such as
>>  (commit (string-append "v" version))
>
> I made this change, but as =(commit version)= since there is no "v" in 
> the release titles.

The commit here will correspond to a git tag, not a github release. I
think it should keep the "v" ;)

>
> I think =guix lint= and =guix style= should now be happy :)
>

Have you tried rebuilding it since?
>
>
Alec Barreto Sept. 10, 2024, 12:13 p.m. UTC | #6
-------- Original Message --------
Subject: Re: [bug#72452] [PATCH] add guile-swayer package
Date: 2024-09-10 8:08 am
 From: mrh57@posteo.net
To: Nicolas Graves <ngraves@ngraves.fr>

On 2024-09-10 4:14 am, Nicolas Graves wrote:
> On 2024-09-09 16:21, mrh57@posteo.net wrote:
> 
>> Thanks again for the feedback.
>> 
>>> 1) (commit field should rather refer to version such as
>>>  (commit (string-append "v" version))
>> 
>> I made this change, but as =(commit version)= since there is no "v" in
>> the release titles.
> 
> The commit here will correspond to a git tag, not a github release. I
> think it should keep the "v" ;)

Ah yes makes sense, whoops!
Change made.

>> I think =guix lint= and =guix style= should now be happy :)
>> 
> 
> Have you tried rebuilding it since?

Yes it successfully builds with these changes under multiple rounds.
Nicolas Graves Sept. 10, 2024, 5:23 p.m. UTC | #7
user guix
usertag 72452 + reviewed-looks-good
thanks

Guix QA review form submission:


Items marked as checked: Lint warnings, Package builds, Commit messages, New package licenses, New package tests, New package synopsis and descriptions
Nicolas Graves Sept. 11, 2024, 3:10 p.m. UTC | #8
I've forgotten to properly check the commit message, it should be:

```
gnu: Add guile-swayer.

* gnu/packages/guile-xyz.scm (guile-swayer): New variable.
```

You can use tempel/yasnippets, guix has some preconfigured commit
messages in (guix source)/etc.

Last nitpick: you can also drop the following change:

  #:use-module (gnu packages haskell-xyz)         ;pandoc
Alec Barreto Sept. 11, 2024, 10:25 p.m. UTC | #9
Nicolas Graves <ngraves@ngraves.fr> writes:

> I've forgotten to properly check the commit message, it should be:
>
> ```
> gnu: Add guile-swayer.
>
> * gnu/packages/guile-xyz.scm (guile-swayer): New variable.
> ```
>
> You can use tempel/yasnippets, guix has some preconfigured commit
> messages in (guix source)/etc.
> Last nitpick: you can also drop the following change:
>
>   #:use-module (gnu packages haskell-xyz)         ;pandoc

Ah thanks, much learned for my next patch.
Changes made.
diff mbox series

Patch

diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm
index 3ce4f6a8d6..5693bbde0f 100644
--- a/gnu/packages/guile-xyz.scm
+++ b/gnu/packages/guile-xyz.scm
@@ -1027,6 +1027,24 @@  (define-public guile2.2-filesystem
     (name "guile2.2-filesystem")
     (inputs (list guile-2.2))))
 
+(define-public guile-swayer
+  (package
+    (name "guile-swayer")
+    (version "0.2.0")
+    (home-page "https://github.com/ebeem/guile-swayer")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/ebeem/guile-swayer")
+             (commit "9c962281f405453fb3770dd0546ef6951c9236dd")))
+       (sha256 (base32 "09c0143q9sm75xp1qz7a7ihdqfwqg4w8nlq0mmnivhvamww775ss"))))
+    (native-inputs (list guile-3.0))
+    (build-system guile-build-system)
+    (synopsis "Extensible Guile bindings for SwayWM")
+    (description "Extensible Guile bindings for SwayWM")
+    (license license:expat)))
+
 (define-public guile-syntax-highlight
   (package
     (name "guile-syntax-highlight")