[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit
From: |
Simon Tournier |
Subject: |
[bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit. |
Date: |
Thu, 17 Aug 2023 20:47:44 +0200 |
Re Maxim,
On Thu, 17 Aug 2023 at 20:16, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
> > On Wed, 16 Aug 2023 at 14:47, Maxim Cournoyer <maxim.cournoyer@gmail.com>
> > wrote:
> >
> >>>> (option '("commit") #t #f
> >>>> (lambda (opt name arg result)
> >>>> - (alist-cons 'ref `(commit . ,arg) result)))
> >>>> + (alist-cons 'ref `(tag-or-commit . ,arg) result)))
> >
> > [...]
> >
> >> (match ref
> >> - (('commit . commit)
> >> + ((or ('commit . commit)
> >> + ('tag-or-commit . commit))
> >
> >> The reason is to standardize the API of (guix pull) and (guix git),
> >> whose procedure had a different expectation for 'ref' objects.
> >
> > My point is that this ’or’ is useless, IIUC. Well, I have removed it in
> > the series fixing the annoyance with the network access of “guix
> > time-machine”.
>
> It isn't, unless you meant after applying further changes :-) You should
> be able to see the problem by reverting that commit and running 'guix
> time-machine --commit=v1.4.0 -- describe', for example.
Yes for sure because you introduced this in guix/scripts/time-machine.scm,
(lambda (opt name arg result)
- (alist-cons 'ref `(commit . ,arg) result)))
+ (alist-cons 'ref `(tag-or-commit . ,arg) result)))
with the previous commit 79ec651a286c71a3d4c72be33a1f80e76a560031.
Well, I feel there is a misunderstanding. :-) And maybe I am missing
something... IIUC, the option parser:
(option '("commit") #t #f
(lambda (opt name arg result)
(alist-cons 'ref `(commit . ,arg) result)))
implemented in guix/scrtips/pull.scm provides a way to construct this
REF. This way should be compliant with the other one in
guix/scripts/time-machine.scm -- at least for being consistent. And I
am missing the reason why you introduced this difference.
If the both option parsers use the same way, then the 'or' is useless.
As I said initially commenting this patch v2 2/3:
--8<---------------cut here---------------start------------->8---
> For compatibility with (guix git) procedures.
>
> * guix/scripts/pull.scm (channel-list): Also accept tag-or-commit tagged
> refspec.
> ---
I am not sure to understand these both changes.
--8<---------------cut here---------------end--------------->8---
Anyway, my other message [1] in #65352 contains the two alternatives
for closing this discussion. :-)
1: https://issues.guix.gnu.org/65352#4
Cheers,
simon
- [bug#64746] [PATCH v2 1/3] git: Clarify commit relation reference in doc., Maxim Cournoyer, 2023/08/15
- [bug#64746] [PATCH v2 3/3] scripts: time-machine: Error when attempting to visit too old commits., Maxim Cournoyer, 2023/08/15
- [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit., Maxim Cournoyer, 2023/08/15
- [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit., Simon Tournier, 2023/08/16
- [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit., Maxim Cournoyer, 2023/08/16
- [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit., Simon Tournier, 2023/08/17
- [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit., Maxim Cournoyer, 2023/08/17
- [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit.,
Simon Tournier <=
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., Maxim Cournoyer, 2023/08/22
- [bug#64746] [PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits., Simon Tournier, 2023/08/23