guix-patches
[Top][All Lists]
Advanced

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

[bug#65352] Fix time-machine and network


From: Maxim Cournoyer
Subject: [bug#65352] Fix time-machine and network
Date: Tue, 22 Aug 2023 22:56:57 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> On Thu, 17 Aug 2023 at 17:42, Maxim Cournoyer <maxim.cournoyer@gmail.com> 
> wrote:
>
>> >                       (match ref
>> > -                       ((or ('commit . commit)
>> > -                            ('tag-or-commit . commit))
>> > +                       (('tag-or-commit . commit)
>
>> Not that channel-list is a public API, so this is effectively changing
>> the contract, no?
>
> Well, the contract is not clearly defined. ;-)
>
> The REF is defined by the docstring of update-cached-checkout,
>
>   REF is pair whose key is [branch | commit | tag | tag-or-commit ] and value
>   the associated data: [<branch name> | <sha1> | <tag name> | <string>].
>   If REF is the empty list, the remote HEAD is used.

Good catch, it seems tag is not covered.

> Therefore, if we want to be compliant with the public API, we also
> need to add 'tag' to the 'or' match case; as I suggested when
> commenting your patch tweaking this part. :-)
>
> Well, from my point of view, the alternative is:
>
>  a)
>                      (match ref
>                        (('tag-or-commit . commit)
>                         (channel (inherit c)
>                                  (url url) (commit commit) (branch #f)))
>                        (('branch . branch)
>                         (channel (inherit c)
>                                  (url url) (commit #f) (branch branch)))
>                        (#f
>                         (channel (inherit c) (url url))))
>
> or b)
>                      (match ref
>                        ((or ('commit . commit)
>                             ('tag-or-commit . commit)
>                             ('tag . commit))
>                         (channel (inherit c)
>                                  (url url) (commit commit) (branch #f)))
>                        (('branch . branch)
>                         (channel (inherit c)
>                                  (url url) (commit #f) (branch branch)))
>                        (#f
>                         (channel (inherit c) (url url)))))
>
> but not ecab937897385fce3e3ce0c5f128afba4304187c. :-)

I was driven by my use case where adding support for tag-or-commit was
enough, but I think it'd be a good idea to cover all the potential ref
types documented in update-cached-checkout, so b) makes sense to me.

-- 
Thanks,
Maxim





reply via email to

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