[PATCH] manifest: move dirs() to manifest

Matt Mackall mpm at selenic.com
Fri Mar 13 17:04:52 CDT 2015


On Fri, 2015-03-13 at 14:18 -0500, Drew Gottlieb wrote:
> # HG changeset patch
> # User Drew Gottlieb <drgott at google.com>
> # Date 1426180563 25200
> #      Thu Mar 12 10:16:03 2015 -0700
> # Node ID 47f1fdab71a77d88ca51d96002a8f3306b10e527
> # Parent  2b7ab29627fd93ca7f5cb838403c2f6c728469bd
> manifest: move dirs() to manifest

This looks fine, but we're really picky about patch granularity.
Please send multiple patches:

- addition of dirs() to manifest
- change of dirs() in context
- addition of hasdir() in manifest
- addition of hasdir() in context
- users of hasdir()

This matters because if you have change X and Y in one patch and change
Y turns out to be subtly wrong, it's not interleaved with X for purposes
of review/acceptance/testing/debugging/bisecting/backout.

A good rule of thumb is that if you find yourself using "and" or bullet
points to explain what you've done, your patch is too big.

Also:

> -            if fn in self._dirs:
> -                # specified pattern is a directory
> -                continue
> -            match.bad(fn, _('no such file in rev %s') % self)
> +            if not self.hasdir(fn):

It's not a big deal here, because this is an error path, but we're going
from a fast Python built-in to three or four layers of slow Python
method lookups/calls in a loop.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list