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

[Augie Fackler]

On Tue, Jul 04, 2017 at 07:41:04PM -0700, Sean Farley wrote:
>
> 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).

It's our intent to turn on email notifications to the list, however:

 * I doubt phabricator gets threading right
 * The emails I've seen don't look very text/plain friendly

So it may be a mixed bag.

> 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.).

Yep. Which is why phabricator is an experiment. We need to kick the
tires a bit before we decide if we want to use it (wether optionally
or as the one true place to do reviews).

Kevin and I are still trying to sort out the configuration issues that
were only discoverable by having it set up, so it's not really
"usable" per se yet (but I hope it will be today).


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel