D5299: phabricator: fallback reading arcanist config files

mharbison72 (Matt Harbison) phabricator at mercurial-scm.org
Fri Feb 22 12:59:38 EST 2019


mharbison72 added inline comments.

INLINE COMMENTS

> phabricator.py:187
> +        paths = [
> +            vfsmod.vfs(encoding.encoding['ProgramData']).join(
> +                 'Phabricator', 'Arcanist', 'config'),

s/.encoding/.environ/ ?

> phabricator.py:200
> +        if vfsmod.vfs(path).exists():
> +            with open(path, 'rb') as f:
> +                config.update(json.load(f))

Should this be using vfs to open, instead of raw open?  Using the vfs layer allows the class that provides posix-like functionality on Windows to be used.

> phabricator.py:290
> +        if not callsign:
> +            return callsign
>      query = callconduit(repo, b'diffusion.repository.search',

It might be clearer to return None, since the function is to fetch the repoid.

REPOSITORY
  rHG Mercurial

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

To: philpep, #hg-reviewers
Cc: mharbison72, mercurial-devel


More information about the Mercurial-devel mailing list