GNU bug report logs - #60791
[PATCH] gnu: services: Add joycond-service.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60791 in the body.
You can then email your comments to 60791 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Thompson, David" <dthompson2 <at> worcester.edu>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: services: Add joycond-service.
Date: Fri, 13 Jan 2023 17:12:26 -0500

[Message part 1 (text/plain, inline)]

Hello Guix,

Joycond is a handy daemon for pairing bluetooth controllers made by
Nintendo. Someone already did the hard work of packaging it, so I
added this simple service to make it easy to use as a system service.

WDYT?

- Dave

[0001-gnu-services-Add-joycond-service.patch (text/x-patch, attachment)]

Message #8 received at 60791 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: 60791 <at> debbugs.gnu.org
Cc: dthompson2 <at> worcester.edu
Subject: [PATCH] gnu: services: Add joycond-service.
Date: Fri, 13 Jan 2023 22:46:19 +0000

Hi,

--8<---------------cut here---------------start------------->8---
+@defvar {Scheme Variable} joycond-service-type
+Service type for the joycond service.
+@end defvar
--8<---------------cut here---------------end--------------->8---

Should be `@defvar joycond-service-type'.

--8<---------------cut here---------------start------------->8---
+(define-record-type* <joycond-configuration>
+  joycond-configuration make-joycond-configuration
+  joycond-configuration?
+  (joycond joycond-configuration-joycond (default joycond)))
--8<---------------cut here---------------end--------------->8---

This could be replaced with define-configuration/no-serialization since
the only field here is a package / file-like object. (see [1], [2] for examples)
I'd prefer the field be called 'package' here. 

--8<---------------cut here---------------start------------->8---
+(define (joycond-shepherd-service config)
+  (let ((joycond (joycond-configuration-joycond config)))
+    (list (shepherd-service
+           (documentation "Run joycond.")
+           (provision '(joycond))
+           (requirement '(bluetooth))
+           (start #~(make-forkexec-constructor
+                     (list #$(file-append joycond "/bin/joycond"))))
+           (stop #~(make-kill-destructor))))))
--8<---------------cut here---------------end--------------->8---

You might prefer match-record here but this is okay as well.


[1]: https://issues.guix.gnu.org/60788
[2]: ddclient-configuration in gnu/services/dns.scm


Cheers,
Bruno

Message #11 received at 60791 <at> debbugs.gnu.org (full text, mbox):

From: "Thompson, David" <dthompson2 <at> worcester.edu>
To: mirai <at> makinata.eu
Cc: 60791 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: services: Add joycond-service.
Date: Fri, 13 Jan 2023 18:50:28 -0500

[Message part 1 (text/plain, inline)]

Hi Bruno,

Thanks for the review!

On Fri, Jan 13, 2023 at 5:46 PM Bruno Victal <mirai <at> makinata.eu> wrote:
>
> Hi,
>
> --8<---------------cut here---------------start------------->8---
> +@defvar {Scheme Variable} joycond-service-type
> +Service type for the joycond service.
> +@end defvar
> --8<---------------cut here---------------end--------------->8---
>
> Should be `@defvar joycond-service-type'.

Oh, okay. Guess I missed that change in convention.  There's lots of
'@defvar {Scheme Variable}' in the manual.

> --8<---------------cut here---------------start------------->8---
> +(define-record-type* <joycond-configuration>
> +  joycond-configuration make-joycond-configuration
> +  joycond-configuration?
> +  (joycond joycond-configuration-joycond (default joycond)))
> --8<---------------cut here---------------end--------------->8---
>
> This could be replaced with define-configuration/no-serialization since
> the only field here is a package / file-like object. (see [1], [2] for examples)
> I'd prefer the field be called 'package' here.

Ahhhh unhygienic macros! Not a fan but I see that this macro is the
preferred thing these days so okay, changed!  I also prefer the field
to be called 'package' so I've changed it! When I looked around at
some other services they used the package name as the field name so I
followed their lead.

> --8<---------------cut here---------------start------------->8---
> +(define (joycond-shepherd-service config)
> +  (let ((joycond (joycond-configuration-joycond config)))
> +    (list (shepherd-service
> +           (documentation "Run joycond.")
> +           (provision '(joycond))
> +           (requirement '(bluetooth))
> +           (start #~(make-forkexec-constructor
> +                     (list #$(file-append joycond "/bin/joycond"))))
> +           (stop #~(make-kill-destructor))))))
> --8<---------------cut here---------------end--------------->8---
>
> You might prefer match-record here but this is okay as well.

If I was destructuring many record fields I'd use it.

Updated patch attached.

Thanks!

- Dave

[0001-gnu-services-Add-joycond-service.patch (application/x-patch, attachment)]

Message #14 received at 60791 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Victal <mirai <at> makinata.eu>
To: "Thompson, David" <dthompson2 <at> worcester.edu>
Cc: 60791 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: services: Add joycond-service.
Date: Sat, 14 Jan 2023 04:47:13 +0000

--8<---------------cut here---------------start------------->8---
joycond-configuration-joycond
--8<---------------cut here---------------end--------------->8---

Doesn't need to be exported, commit message could be amended as well.


--8<---------------cut here---------------start------------->8---
+@table @asis
+@item @code{joycond} (default: @code{joycond})
+The joycond package to use.
+@end table
+@end deftp
--8<---------------cut here---------------end--------------->8---

@code should be updated to the new `package' field name.


Overall LTGM.


Cheers,
Bruno

Message #19 received at 60791-done <at> debbugs.gnu.org (full text, mbox):

From: "Thompson, David" <dthompson2 <at> worcester.edu>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 60791-done <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: services: Add joycond-service.
Date: Sat, 14 Jan 2023 08:00:15 -0500

On Fri, Jan 13, 2023 at 11:47 PM Bruno Victal <mirai <at> makinata.eu> wrote:
>
> --8<---------------cut here---------------start------------->8---
> joycond-configuration-joycond
> --8<---------------cut here---------------end--------------->8---
>
> Doesn't need to be exported, commit message could be amended as well.
>
>
> --8<---------------cut here---------------start------------->8---
> +@table @asis
> +@item @code{joycond} (default: @code{joycond})
> +The joycond package to use.
> +@end table
> +@end deftp
> --8<---------------cut here---------------end--------------->8---
>
> @code should be updated to the new `package' field name.
>
>
> Overall LTGM.

Thanks for catching my silly late-evening fixup oversights.  Fixed all
of that and pushed!

- Dave

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.