[PATCH 08 of 10 V2] sparse-revlog: introduce native (C) implementation of slicechunktodensity

Yuya Nishihara yuya at tcha.org
Mon Nov 19 07:14:31 EST 2018


On Mon, 19 Nov 2018 10:19:55 +0100, Boris FELD wrote:
> >> +	double targetdensity = 0.5; /* min density to achieve */
> >> +	Py_ssize_t mingapsize = 0;  /* threshold to ignore gaps */
> > Nit: perhaps these default values, 0.5 and 0, wouldn't be used since function
> > arguments aren't optional.
> I updated the code to set them both to 0. Should we leave them
> non-initialized?

I don't care much about that. 0 should be fine.

> > This patch looks good to me.
> >
> > As a follow up, can you somehow rewrite the doctests in deltas.py to run with
> > the C implementation? Since _testrevlog() isn't backed by the index object,
> > C functions aren't covered by the doctests. That's unfortunate.
> That won't be easy, the doctest use a mock python class to emulate the
> revlog. On the other hand, the C code expects the real object.

Can't they be extracted to unit tests? I don't think doctests fit well to
these somewhat complex functions.


More information about the Mercurial-devel mailing list