phillco abandoned this revision.
phillco added a comment.
I've split this into https://phab.mercurial-scm.org/D1056,
https://phab.mercurial-scm.org/D1057, https://phab.mercurial-scm.org/D1058,
https://phab.mercurial-scm.org/D1059, https://phab.mercurial-scm.org/D1060, so
abandoning this
martinvonz requested changes to this revision.
martinvonz added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> filemerge.py:617-624
> +if isinstance(fcd, context.overlayworkingfilectx) and inworkingdir:
> +# If the backup file is to be in the
phillco updated this revision to Diff 2088.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D674?vs=1989=2088
REVISION DETAIL
https://phab.mercurial-scm.org/D674
AFFECTED FILES
mercurial/context.py
mercurial/filemerge.py
CHANGE DETAILS
diff --git
phillco added a comment.
> One extra comment: maybe include some "why" as well as "what" in your
commit message. :)
Sorry I missed this, the comment is very valid. Will send a new version.
INLINE COMMENTS
> martinvonz wrote in filemerge.py:611
> Isn't repo.wjoin(fcd.path()) the same
martinvonz added inline comments.
INLINE COMMENTS
> filemerge.py:611
> +from . import context
> +back = scmutil.origpath(ui, repo, repo.wjoin(fcd.path()))
>
Isn't repo.wjoin(fcd.path()) the same thing as _workingpath(repo, fcd) inlined?
You call the same thing further down, so why
martinvonz added inline comments.
INLINE COMMENTS
> context.py:701-705
> +class abstractfilectx(object):
> +def data(self):
> +raise NotImplementedError
> +
> +class basefilectx(abstractfilectx):
abstractfilectx doesn't seem referenced anywhere else, so just revert this
part? Maybe
durin42 added a comment.
One extra comment: maybe include some "why" as well as "what" in your commit
message. :)
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D674
To: phillco, #hg-reviewers
Cc: durin42, sid0, martinvonz, mercurial-devel
phillco updated this revision to Diff 1874.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D674?vs=1871=1874
REVISION DETAIL
https://phab.mercurial-scm.org/D674
AFFECTED FILES
mercurial/context.py
mercurial/filemerge.py
CHANGE DETAILS
diff --git
phillco updated this revision to Diff 1871.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D674?vs=1830=1871
REVISION DETAIL
https://phab.mercurial-scm.org/D674
AFFECTED FILES
mercurial/context.py
mercurial/filemerge.py
CHANGE DETAILS
diff --git
phillco planned changes to this revision.
phillco added a comment.
Putting back in my queue -- still have two comments of Martin's to respond to.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D674
To: phillco, #hg-reviewers
Cc: sid0, martinvonz, mercurial-devel
phillco updated this revision to Diff 1830.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D674?vs=1724=1830
REVISION DETAIL
https://phab.mercurial-scm.org/D674
AFFECTED FILES
mercurial/context.py
mercurial/filemerge.py
CHANGE DETAILS
diff --git
phillco added inline comments.
INLINE COMMENTS
> martinvonz wrote in filemerge.py:744
> Can we not simply make arbitraryfilectx.cmp() something like
>
> def cmp(self, fctx):
> if isinstance(fctx, arbitraryfilectx):
> return filecmp.cmp(self.path(), fctx.path())
> return
martinvonz added inline comments.
INLINE COMMENTS
> phillco wrote in filemerge.py:744
> Yes, and it's a bigger impact than past changes of this nature, since each
> merged file, and its backup file, will be read again. So I think we need some
> way to reintroduce this fast-path for two
phillco added inline comments.
INLINE COMMENTS
> martinvonz wrote in filemerge.py:744
> The documentation for filecmp.cmp() says
>
> Unless shallow is given and is false, files with identical os.stat()
> signatures are taken to be equal.
>
> Are we losing out on that optimization with this
phillco added a subscriber: sid0.
phillco added inline comments.
INLINE COMMENTS
> martinvonz wrote in test-dirstate-race.t:85
> Why did this change?
It's caused by the addition of this block into `abstractfilectx`.
if isinstance(fctx, abstractfilectx):
return self.data() !=
martinvonz added inline comments.
INLINE COMMENTS
> context.py:700
>
> -class basefilectx(object):
> +class abstractfilectx(object):
> +def data(self):
Feel like checking out the "abc" module and see if that's helpful here?
> context.py:854
> """
> if fctx._customcmp:
>
phillco updated this revision to Diff 1724.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D674?vs=1704=1724
REVISION DETAIL
https://phab.mercurial-scm.org/D674
AFFECTED FILES
mercurial/context.py
mercurial/filemerge.py
tests/test-dirstate-race.t
phillco created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D674
AFFECTED FILES
mercurial/context.py
mercurial/filemerge.py
tests/test-dirstate-race.t
CHANGE
18 matches
Mail list logo
