[Commented On] D8552: fix: use context to fetch mergestate instead of loading it directly

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D8552#127900 <https://phab.mercurial-scm.org/D8552#127900>, @durin42 wrote:
  
  > In D8552#127895 <https://phab.mercurial-scm.org/D8552#127895>, @martinvonz wrote:
  >
  >> I think I'd prefer to leave code that doesn't use `overlayworkingctx` unchanged. I don't see any reason to get the merge state from the context if we know that the context is a regular `workingctx`.
  >
  > I'd rather it was globally-consistent that if you need a mergestate, you load it via a context, so that more code will implicitly be right for in-memory operations.
  
  As long as we do `repo[None].mergestate()` it will not be more or less right than before. We'd at least want to refactor the code so it creates a `wctx` instance early on and then reuse that as much as possible, so we could later replace the `wctx = repo[None]` by `wctx = overlayworkingctx(...)`. In cases like this one (checking for unresolved merge conflicts), it will never make sense to do that on an `overlayworkingctx`. I'm very much in favor of making more things work "in memory" where that's reasonable, but I'd prefer to leave callers unchanged until we actually try that in earnest. I think we have a similar situation with the dirstate and I don't think we've tried to hide all access to the dirstate bethind the context objects.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8552/new/

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

To: durin42, #hg-reviewers
Cc: martinvonz, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200519/598f7326/attachment-0002.html>