D7628: config: move reading of defaultrc/ files from ui to rcutil
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Mon Dec 23 12:28:16 EST 2019
indygreg added a comment.
The patch seems functionally correct. But I have a question about whether this is the desired approach. See inline comment.
> + 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.
CHANGES SINCE LAST ACTION
To: martinvonz, #hg-reviewers
Cc: indygreg, mercurial-devel
More information about the Mercurial-devel