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

[pulkit (Pulkit Goyal)]

pulkit added a comment.


  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()`.

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/20200620/51fa3e65/attachment-0002.html>