[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.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [bug#74801] [PATCH] gnu: home: services: Add home-mpv-service-type.,
Tomas Volf <=