[Commented On] D8631: localrepo: add writerequirements() and route requires writing through it

marmoute (Pierre-Yves David) phabricator at mercurial-scm.org
Sun Jun 21 15:55:13 UTC 2020


marmoute added a comment.


  In D8631#129152 <https://phab.mercurial-scm.org/D8631#129152>, @pulkit wrote:
  
  > In D8631#129134 <https://phab.mercurial-scm.org/D8631#129134>, @marmoute wrote:
  >
  >> In D8631#129129 <https://phab.mercurial-scm.org/D8631#129129>, @pulkit wrote:
  >>
  >>> In D8631#129003 <https://phab.mercurial-scm.org/D8631#129003>, @marmoute wrote:
  >>>
  >>>> "callers directly calling scmutil.writerequires() are **once**
  >>>> where" Do you mean "only"  ?
  >>>
  >>> Yes.
  >>>
  >>>> The patch seems to go in the right direction, but the function seems rarely needed enough that is probably worth not making it a method. Could we update the function to take a repo (instead of vs) and act accordingly ?
  >>>
  >>> There are some places where we don't have a repo object. So those we route to `scmutil.writerequires` which accepts a vfs. Did you mean updating `scmutil.writerequires` or updating the new method I added?
  >>
  >> what I mean is "not having a new method on repo"
  >
  > I tried that. It looks a bit ugly because that function will then call `repo._writerequires()`.
  
  I don't get it, why do we need to have a repo method at all ?

REPOSITORY
  rHG Mercurial

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

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

To: pulkit, #hg-reviewers
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200621/90d8247e/attachment-0002.html>


More information about the Mercurial-patches mailing list