[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
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., (continued)
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., Maxim Cournoyer, 2023/08/15
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., Ludovic Courtès, 2023/08/15
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., Simon Tournier, 2023/08/16
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., Maxim Cournoyer, 2023/08/16
- [bug#65352] Fix time-machine and network, Simon Tournier, 2023/08/17
- [bug#65352] [PATCH 1/2] guix: git: Fix the procedure reference-available?., Simon Tournier, 2023/08/17
- [bug#65352] [PATCH 2/2] scripts: pull: Remove unused reference pair., Simon Tournier, 2023/08/17
- [bug#65352] Fix time-machine and network, Maxim Cournoyer, 2023/08/17
- [bug#65352] Fix time-machine and network, Simon Tournier, 2023/08/17
- [bug#65352] Fix time-machine and network,
Maxim Cournoyer <=
- [bug#65352] Fix time-machine and network, Simon Tournier, 2023/08/23
- [bug#65352] Fix time-machine and network, Maxim Cournoyer, 2023/08/23
- [bug#65352] Fix time-machine and network, Ludovic Courtès, 2023/08/21
- [bug#65352] Fix time-machine and network, Maxim Cournoyer, 2023/08/21
- [bug#65352] Fix time-machine and network, Ludovic Courtès, 2023/08/22
- [bug#65352] Fix time-machine and network, Maxim Cournoyer, 2023/08/22
- [bug#65352] Fix time-machine and network, Ludovic Courtès, 2023/08/21