| Message ID | 20190109025914.247473-1-matvore@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | support for filtering trees and blobs based on depth | expand |
> This applies suggestions from Jonathan Tan and Junio. These are mostly > stylistic and readability changes, although there is also an added test case > in t/t6112-rev-list-filters-objects.sh which checks for the scenario when > filtering which would exclude a blob, but the blob is given on the command > line. > > This has been rebased onto master, while the prior version was based on next. > > Thank you, Thanks, these 2 patches are Reviewed-by: me. Your approach in the 2nd patch makes more sense, and I checked that both oidset_insert() and oidset_remove() return 1 when the element in question was in the set (prior to invocation of the function), so that works.
Jonathan Tan <jonathantanmy@google.com> writes: >> This applies suggestions from Jonathan Tan and Junio. These are mostly >> stylistic and readability changes, although there is also an added test case >> in t/t6112-rev-list-filters-objects.sh which checks for the scenario when >> filtering which would exclude a blob, but the blob is given on the command >> line. >> >> This has been rebased onto master, while the prior version was based on next. >> >> Thank you, > > Thanks, these 2 patches are Reviewed-by: me. > > Your approach in the 2nd patch makes more sense, and I checked that both > oidset_insert() and oidset_remove() return 1 when the element in > question was in the set (prior to invocation of the function), so that > works. This is turning out to be messier than I like. The topic is tangled with too many things in flight and I think I reduced its dependencies down to nd/the-index and sb/more-repo-in-api plus then-current tip of master (and that is why it is based on a1411cecc7), but it seems that it wants a bit more than that; builtin/rebase.c at its tip does not even compile, so I'll need to wiggle the topic before it can go to 'next'. And worse yet, it seems that filter-options-should-use-plain-int topic depends on this topic in turn as it wants to use LOFC_TREEE_DEPTH.
Junio C Hamano <gitster@pobox.com> writes: > This is turning out to be messier than I like. > > The topic is tangled with too many things in flight and I think I > reduced its dependencies down to nd/the-index and > sb/more-repo-in-api plus then-current tip of master (and that is why > it is based on a1411cecc7), but it seems that it wants a bit more > than that; builtin/rebase.c at its tip does not even compile, so > I'll need to wiggle the topic before it can go to 'next'. Half false alarm. I do need to wiggle the topic, but that was not because the choice of base was bad. It was that nd/the-index plus sb/more-repo-in-api had semantic merge conflicts with the then-current master. > And worse yet, it seems that filter-options-should-use-plain-int > topic depends on this topic in turn as it wants to use > LOFC_TREEE_DEPTH. This part is still true. The scaling-factor-over-the-wire topic does need to be rebuilt on top of this one.
On 2019/01/15 15:41, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> This is turning out to be messier than I like. >> >> The topic is tangled with too many things in flight and I think I >> reduced its dependencies down to nd/the-index and >> sb/more-repo-in-api plus then-current tip of master (and that is why >> it is based on a1411cecc7), but it seems that it wants a bit more >> than that; builtin/rebase.c at its tip does not even compile, so >> I'll need to wiggle the topic before it can go to 'next'. > Half false alarm. I do need to wiggle the topic, but that was not > because the choice of base was bad. It was that nd/the-index plus > sb/more-repo-in-api had semantic merge conflicts with the then-current > master. If I understand right, this is a product of the fact that you had to merge these branches together and base my change on top of them, and that is a result of that fact that I didn't work on top of master for the first iterations of the patch. Sorry about that. My last re-roll was based on master (commit 77556354) but I guess before I sent that version of the patch set I had already done some damage by working off of next for the earlier patches. I think my last version of the patch was fine since it was based off master. Let me know if I've misunderstood. >> And worse yet, it seems that filter-options-should-use-plain-int >> topic depends on this topic in turn as it wants to use >> LOFC_TREEE_DEPTH. > This part is still true. The scaling-factor-over-the-wire topic > does need to be rebuilt on top of this one. This seems like a easier problem to understand, but I'm not sure how to avoid this issue in the future. Thanks, Matt
Matthew DeVore <matvore@comcast.net> writes: > This seems like a easier problem to understand, but I'm not sure how > to avoid this issue in the future. Sorry---I was mostly venting and it was not productive. There isn't much individual contributors can do by themselves, other than choosing the right place to base their topics on and communicate it accurately when sending the patches to the list. I think I made sure that all the topics in master..pu that have tricky dependencies are at least buildable (which involved a few topics to be rebased on the right commit, sometimes rebuilding the base that is a merge of a few topics in flight), so hopefully we are in good shape now. Thanks.