[bug#48729] [PATCHES v9 ALL/25]: Bitmask

[Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org>]

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

Hi Raghav,

I spent an embarrasingly long time trying to get this to work 
under Sway (I'm sure it works fine under a GNOME VM, that's not 
the point :-p) but had to give up.  It's not a blocker.

And with that, I think only have two more remarks!  You don't need 
to send a new revision to review.

> @subsubheading Bitmask
> +
> +@defvr {Scheme Variable} bitmask-service-type
> +A service type for the @uref{https://bitmask.net, Bitmask} VPN 
> application.
> +It makes the application ready for use.
> +@end defvr

Thanks for adding some docs!  Unfortunately I learnt nothing :-)

Documentation should teach something new, for example:

  “This service type makes the @uref{https://bitmask.net, Bitmask} 
  VPN
   client available to all users as a system package, and installs 
   the
   global system Polkit permission rules currently required for 
   users
   to run it.

   It will @emph{not} start a Polkit agent for you, as most 
   desktop
   environments provide their own.  Users who don't use such a 
   desktop
   environment must run their own agent in order to use Bitmask.”

> +   (default-value bitmask)

That's OK if there's no way this service will need to be extended 
in future, which IIRC was the case, since eventually it should 
even be removed.

*Otherwise* I'd recommend wrapping this in an extensible 
 (bitmask-configuration (bitmask PACKAGE)) record.

Thanks, again, to you both for all your effort.

Kind regards,

T G-R

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