On Wed, 24 Jul 2024 at 12:48, Christian Couder <christian.couder@xxxxxxxxx> wrote: > > On Wed, Jul 24, 2024 at 7:12 AM Chandra Pratap > <chandrapratap3519@xxxxxxxxx> wrote: > > > > On Tue, 23 Jul 2024 at 22:39, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > > > > - Rename tests according to unit-tests' conventions > > > > - remove 'pq_test_main()' from reftable/reftable-test.h > > > > > > > > CI/PR for v5: https://github.com/gitgitgadget/git/pull/1745 > > > > > > By the way, I still haven't got any answer to a question I asked > > > long ago on this series, wrt possibly unifying this pq and another > > > pq we already use elsewhere in our codebase. If we are butchering > > > what we borrowed from elsewhere and store in reftable/. directory > > > and taking responsibility of maintaining it ourselves, we probably > > > should consider larger refactoring and cleaning up, and part of it > > > we may end up discarding this pq implementation, making the unit > > > testing on it a wasted effort. > > I agree it might have been better to start by replacing the pq > implementation in reftable/ with our own first, as there would be no > need for this patch series, but Chandra's GSoC is about replacing the > unit test framework in reftable/ with our own which is still valuable. > And I think that at this point it is just simpler to not get > distracted by replacing the pq implementation now. It's also not like > changing the unit test framework would make replacing the pq > implementation harder. > > > I did talk about this with Patrick and Christian on a private slack channel > > a few weeks ago and here is how that conversation went: > > > > Me: Hey, I wanted to talk about the message from Junio the other day. > > It is true that through this project, we are modifying the reftable directory > > to a point that it is no longer easily usable by someone from outside. If > > that is the direction we want to take, wouldn't it make more sense to get > > rid of reftable/pq.{c, h} altogether and use Git's prio-queue instead? > > > > Christian: Yeah, I think the direction the Git project wants to take is to > > integrate the reftable code more and more with the Git code. On the other > > hand, there are libification projects which are trying to split parts of the > > Git code into libraries usable by other projects. But I don't think each of > > these libraries should have their own test framework, their own prio-queue > > implementation, their own string implementation, etc. So, even if I am not > > sure about the end result, I think it would be ok to modify the reftable code > > so that it uses the Git's prio queue and maybe other Git data structures. > > But I'd like Patrick to confirm, and the list to agree to this. So I'd > > rather wait > > until Patrick is back from his vacation before doing things like replacing > > reftable/pq.{c, h} with Git's prio-queue. > > Yeah, if it had been discussed and agreed on earlier, I think > replacing the pq implementation would have made sense. Now I think > it's a bit late at this stage in Chandra's GSoC to go in this > direction though. I think it's better if he can focus on finishing to > replace the unit test framework. I'm actually more or less done with porting most of the reftable tests to the unit test framework and it shouldn't be long before patches for rest of the tests see the mailing list, so I can definitely make time for other endeavors like these. The only thing that I think is holding us back from making a change like what Junio suggests is 'how far are we willing to butcher our copy of reftable/?' From what Patrick said earlier, I think the answer to that is 'it is fine to modify reftable/ tests because they don't need to be uniform across all implementations of reftable, but modifying other parts to be more dependent on Git's internals is a no-go.'
