Re: [PATCH v3 4/4] for-each-ref: avoid filtering on empty pattern

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>>
>>> Thanks I'd missed that discussion. I see that at the end of that
>>> discussion Junio was concerned that the proposed "" did not account
>>> for "refs/worktrees/$worktree/*" [1] - how has that been resolved?
>>
>> Ah, that is an excellent point.
>> ...
>> The use of dashed-options to include hierachies that are by default
>> excluded (e.g. "--include-root-refs" and "--include-worktree-refs")
>> feels limiting, but should be sufficient for our needs, both current
>> (i.e. I want to see HEAD and FETCH_HEAD) and in the immediate future
>> (i.e. I want to see worktree refs from that worktree), and I can buy
>> that as a good alternative solution, at least in the shorter term.
>>
>> I still worry that it may make introducing the negative ref patterns
>> harder, though.  How does --include-worktree-refs=another to include
>> the worktree refs from another worktree in refs/worktrees/another
>> interact with a negative pattern that was given from the command
>> line that overlaps with it?  Whatever interaction rules we define,
>> can we easily explain it in the documentation?
>>
>> Just like "an empty string tells Git to include everything" is a
>> perfectly reasonable approach if we plan to never allow
>> refs/worktrees/ hierarchy, "dashed-options for selected hierarchies"
>> is a perfectly reasonable approach if we plan to never allow
>> negative limit patterns, I suspect.  We should stop complexity at
>> some point, and the decision to never support negative limit
>> patterns might be the place to draw that line.  I dunno.
>
> For now, let's block the kn/for-all-refs topic until we figure out
> the UI issue.  Which means this (and the reftable integration that
> started to depend on it) will not be in the upcoming release.
>

I think it makes sense to remove the kn/for-all-refs topic for now.

I also think that the reftable integration branch can go forward though,
since the difference between v2 and v3 of that series was simply that
v3 was rebased on top of kn/for-all-refs. So we can continue using v2.

> FWIW, I am leaning towards "a set of narrowly targetted command line
> options (like "--include-root-refs")" approach over "a empty string
> defeats the built-in default of 'refs/' limit".

I think this was what the earlier discussion in the RFC series was too,
but Phillip definitely helped consolidate the point.

I'll send a new version of this patch series with `--include-root-refs`
option and we can discuss on top of that.

Thanks all!

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux