D444: merge: move some of the logic in batchget() to workingfilectx

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Thu Aug 31 03:29:41 EDT 2017


martinvonz added inline comments.

INLINE COMMENTS

> context.py:1971-1973
> +        if self._repo.wvfs.isdir(self._path) and not self._repo.wvfs.islink(
> +                self._path):
> +            self._repo.wvfs.removedirs(self._path)

self.repo.wvfs gets repeated often enough that it's probably worth extracting to a local variable

> context.py:1975-1977
> +        dirname = os.path.dirname(self._path)
> +        if not self._repo.wvfs.isdir(dirname):
> +            self._repo.wvfs.makedirs(dirname)

I'd prefer to see this addition in the patch that needs it. At this point it's not clear what this is for.

Also, with this code here, it seems a little unnecessary for the block above to delete parent directories if we're just going to create them again here. Am I reading that right?

> merge.py:1153-1154
>                          raise
> -
> -            if repo.wvfs.isdir(f) and not repo.wvfs.islink(f):
> -                repo.wvfs.removedirs(f)
> +            wctx[f].clearunknown()
>              wctx[f].write(fctx(f).data(), flags, backgroundclose=True)
>              if i == 100:

nit: wctx[f] involves enough work that it's probably worth extracting it to a variable?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D444

To: phillco, #hg-reviewers
Cc: martinvonz, mercurial-devel


More information about the Mercurial-devel mailing list