[PATCH 10 of 10] phabricator: do not read a same revision twice

Sean Farley sean at farley.io
Tue Jul 4 22:41:04 EDT 2017


Jun Wu <quark at fb.com> writes:

> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1499219548 25200
> #      Tue Jul 04 18:52:28 2017 -0700
> # Node ID 6f1f74ecc788edf66ab9b0e8717724b97666f037
> # Parent  0e2c9cf54e09bacb4a019bc51693a9fecf1051a3
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 6f1f74ecc788
> phabricator: do not read a same revision twice

This series is generic for any phabricator instance, correct? I mean,
we're not hardcoding any assumptions or things like that.

> It's possible to set up non-linear dependencies in Phabricator like:
>
>   o   D4
>   |\
>   | o D3
>   | |
>   o | D2
>   |/
>   o   D1
>
> The old `phabread` code will print D1 twice. This patch adds de-duplication
> to prevent that.
>
> Test Plan:
> Construct the above dependencies in a Phabricator test instance and make
> sure the old code prints D1 twice while the new code won't.

Is part of this test going to include sending a (threaded) email with
the patch? Hopefully, comments from phabricator will get mirrored to the
list as well so that subscribers can see the reviews (it seems that
mailing list -> phabricator doesn't exist currently).

There is another issue I worry a lot about with any new review system
and that is keeping the discussion archives on the mailing list. Having
more than one place of archives is completely untenable in my experience
(do I search phabricator? the mailing list? something else?).
Furthermore, the mailing list is archived by many entities right now.
Having a hosted solution runs the risk of a single point of failure
(server dying, etc.).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170704/a9e1659f/attachment.sig>


More information about the Mercurial-devel mailing list