guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#74801] [PATCH] gnu: home: services: Add home-mpv-service-type.


From: Tomas Volf
Subject: [bug#74801] [PATCH] gnu: home: services: Add home-mpv-service-type.
Date: Fri, 02 May 2025 18:40:58 +0200
User-agent: Gnus/5.13 (Gnus v5.13)

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Tomas,
>
> Tomas Volf <~@wolfsden.cz> writes:
>
> [...]
>
>>>> +
>>>> +;;;
>>>> +;;; Basic types.
>>>> +;;;
>>>> +(define (serialize-type/boolean field-name value)
>>>
>>> Nitpick: it's more common in the code base to name serializers as
>>> 'serialize-boolean'; it'd be nicer to stick to that naming style, for
>>> consistency.
>>
>> But it does follow the same pattern.  The type is named type/boolean.
>> So the pattern of serialize-$TYPE results in serialize-type/boolean.
>> Without the type/ prefix I would run into collisions, since I need a
>> predicate for an integer (type/integer?), but integer? is already a
>> procedure doing something else.  I am not sure I want to shadow it.
>>
>> Hm, would it make it better for you if I replaced the type/ prefix with
>> mpv/ prefix?  So mpv/integer, instead of type/integer?  That would
>> result in serialize-mpv/boolean, which might be better in your eyes?
>
> I think that'd be more precise naming yes, if there's a valid reason
> that mpv/integer? != integer? (which I'm sure there is since you went to
> the trouble of defining it!)

Well, mpv/integer is basically an integer with additional range check.

--8<---------------cut here---------------start------------->8---
(define (mpv/integer? n)
  ;; We assume integer is a signed 32bit number.
  (and-let* (((integer? n))
             ((>= n (* -1 (expt 2 (1- 32)))))
             ((<= n (1-   (expt 2 (1- 32))))))))
--8<---------------cut here---------------end--------------->8---

On the other hand, for e.g. mpv/boolean there is nothing extra to check,
so it is just a matter of consistency to have all "types" sharing the
same naming pattern.

I have renamed all the type/ to mpv/, so hopefully you will find the new
version better. :)

>>>
>>> Could you send a v2 with the above taken into consideration?
>>
>> Once I know what to do with the type/..., will send. :)
>
> Sorry for the delay.  Now you know :-).

And sorry for the delay from my side. :)  I really appreciate you taking
the time to do the review, and have now sent a v2 hopefully addressing
all your concerns.

Have a nice day,
Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]