On Wed, Apr 21, 2021 at 1:21 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > >> I don't see the point of having re-rolls of this topic while the test > >> changes topic it's based on hasn't finished > >> marking/splitting/refactoring the various tests that do and don't depend > >> on the file backend. > >> > >> At least when I review it I'm just left with going in circles digging > >> into one of those failing test, figuring out if it's really > >> refs/files-backend.c specific or not etc., and as long as we can't turn > >> on GIT_TEST_REFTABLE=true in CI as part of this series I don't see a > >> path to making it advance to next->master. > > > > The point of posting updates is to garner feedback, especially from > > people familiar with the Git code itself. > > So you agree that we should make the tests pass first, then shouldn't > these me marked as RFC/PATCH? I added RFC to the patch introducing reftable-backend.c I consider the code under reftable/*.c to be of production quality, even if it's stylistically different from the git code. > IOW a large part of the feedback you're looking for is already part of > the codebase. Nobody can keep all of it in their head, but we've encoded > all the tricky edge cases we could think of in the tests. That's disappointing. I know that I can just fix test failures until there are none left, but it's not a terribly efficient method if there is someone who knows how this works. Is there no other way to solicit feedback? > I.e. reaching some consensus on things like whether > GIT_TEST_REFTABLE=true passing under CI being a hard-prereq for this > series or not is surely one of the first things to sketch out before > figuring out how to move this forward. I've had conflicting feedback about this, but it's ultimately not my call to make. Jonathan Nieder suggested that this was asking too much, and it would be OK to have a credible starting point where the effort to address remaining test failures could be shared across multiple people. Jun also expressed he didn't want to put all the work on my shoulders, but so far I've not received much help. > In v6 I noted that t5510-fetch.sh had a segfault[4], you said you'd > check it out, and reading your cover letter nothing stood out about > that, so I assumed it was sorted out somehow. I fixed a number of segfaults, so it's quite likely (I did see the freed transaction BUG). One of the problems of working with a large pending series like this, is that it is a project in itself, and keeping track of which change fixes which bug is something I really need version control for, but the frequent rebasing/squashing erases this kind of information. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
