indygreg added a comment.

  The patch seems functionally correct. But I have a question about whether 
this is the desired approach. See inline comment.

INLINE COMMENTS

> rcutil.py:102
> +                    default_entries.append((section, name, value, source))
> +        _rccomponents = [(b'items', default_entries)]
> +

Do you think it would be better to change the return type to expose a 
`resource` type, which the caller can resolve accordingly? Or maybe do away 
with leaving it up to the caller to open and read paths? It just feels weird 
for some I/O to be performed in this function while some I/O is deferred to the 
caller.

But I don't have this code paged in, so maybe my comment is off base.

REPOSITORY
  rHG Mercurial

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

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

To: martinvonz, #hg-reviewers
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to