On Mon, Nov 18, 2024 at 07:42:58AM +0100, Patrick Steinhardt wrote:
> > > Rubén's review went through all of the patches and his findings have
> > > been addressed.
> >
> > Yes, this iteration looks good to me.
> >
> > Two thoughts about the merge:
> >
> > First, I'm concerned that we may not have sufficiently documented how
> > contributors should proceed to prevent new leaks when submitting
> > patches, and perhaps avoid some unnecessary noise on the list. I
> > reviewed Documentation/SubmittingPatches and didn't see any mention
> > about it. Perhaps it would be helpful to add a note about
> > SANITIZE=leak. I'm unsure if we want to be explicit about this,
> > though.
>
> Nothing really changes with this series -- we already required code to
> be leak free beforehand, just not in all of our tests. But in any case,
> providing pointers for how to check for leaks somewhere could be helpful
> indeed.
>
> I think that can happen outside of this series though, also because I'm
> not quite sure where to slot this in.
Re-reading Documentation/SubmittingPatches, I think the recommendation
we already have might be sufficient:
Pushing to a fork of https://github.com/git/git will use their CI
integration to test your changes on Linux, Mac and Windows. See
the <<GHCI,GitHub CI>> section for details.
Anyway, of course it can happen outside this series. For me, the
series is ready to be merged.
