[Commented On] D8524: phabricator: add .arcconfig to help messages and comments (issue6331)

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D8524#134326 <https://phab.mercurial-scm.org/D8524#134326>, @sfink wrote:
  
  > In D8524#134322 <https://phab.mercurial-scm.org/D8524#134322>, @mharbison72 wrote:
  >
  >> The documentation updates look good to me, but there are still some code changes that were probably not intentional, and the commit message probably needs an update.
  >
  > I updated the commit message to "add .arcconfig to help messages and comments", which seems accurate.
  
  I meant this, which is still in the summary section
  
    Wrapping localrepo's loadhgrc() was not working for me because it is too late to wrap loadhgrc when the extension is loaded.
  
  > This is now the only code change (removing the explicit result variable and instead just using cfg to tell whether we updated any configuration.) This seems to better match the intent documented in mercurial/localrepo.py:
  >
  >   Returns a bool indicating whether any additional configs were loaded.
  >
  > The only difference with the redundant result variable would be if you loaded in a .arcconfig that did not contain either of the two settings looked at here, and in that case it seems like it would be better to return False.
  > What do you think?
  
  I understand now.  Yeah this code is better.  Usually we try to split unrelated documentation updates into a separate commit though, so can you `hg split` this (and get rid of the differential url in one of theme)?

REPOSITORY
  rHG Mercurial

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

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

To: sfink, mharbison72, marmoute
Cc: marmoute, Kwan, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200829/da863866/attachment-0002.html>