Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 03 Sep 2019 14:06:03 -0000
From:      Enji Cooper <yaneurabeya@gmail.com>
To:        rgrimes@freebsd.org
Cc:        Enji Cooper <ngie@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head@freebsd.org
Subject:   Re: svn commit: r345707 - in head: lib/clang lib/libc++ lib/libc++experimental lib/libc++fs lib/libc/tests/stdlib lib/libclang_rt lib/libcxxrt lib/libgcc_eh lib/libomp lib/ofed/libibnetdisc share/mk us...
Message-ID:  <E790A24C-E935-45CF-A957-98FB39852F31@gmail.com>
In-Reply-To: <201903300309.x2U39DtW002526@gndrsh.dnsmgr.net>
References:  <201903300309.x2U39DtW002526@gndrsh.dnsmgr.net>

next in thread | previous in thread | raw e-mail | index | archive | help

> On Mar 29, 2019, at 8:09 PM, Rodney W. Grimes =
<freebsd@gndrsh.dnsmgr.net> wrote:
>=20
>> Author: ngie
>> Date: Fri Mar 29 18:43:46 2019
>> New Revision: 345707
>> URL: https://svnweb.freebsd.org/changeset/base/345707
>>=20
>> Log:
>>  Revert r345706: the third time will be the charm
>>=20
>>  When a review is closed via Phabricator it updates the patch =
attached to the
>>  review. I downloaded the raw patch from Phabricator, applied it, and =
repeated
>>  my mistake from r345704 by accident mixing content from D19732 and =
D19738.
>=20
> Which, arguable is a feature or mis feature depending on the point
> of view.  I do not like it when I go to look at someone elses
> committed code siting a review, as I want to actually see what
> it was that was committed.  You can find the pre-commit diff,
> but it takes a bit of probling.  The upside is you can get
> both diffs from the same place and diff the diffs :-)
>=20
>>  For my own personal sanity, I will try not to mix reviews like this =
in the
>>  future.
>=20
> :-)  Been there, almost did that too.
> Pre commit last minute svn diff saved me.

=E2=80=A6

This is why I=E2=80=99m doing the following from here on out:

$ arc patch
$ svn ci

Unfortunately svn doesn=E2=80=99t support all of the niceties of =E2=80=9C=
arc land=E2=80=9D. Otherwise, I would have used that.

The Facebook version of =E2=80=9Carc land=E2=80=9D (before their new =
non-public variation) supported verifying diffs in local repos vs =
Phabricator to make sure that the diff content was consistent/correct.

* Pro: it would catch issues like what I did the first time.
* Con: I couldn=E2=80=99t make last minute changes (I would need to =
resubmit the change and have it re-reviewed, which I argue is a good =
feature).

Just some food for thought. For now, arc patch/svn ci works for me.

Thanks :),
-Enji=





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E790A24C-E935-45CF-A957-98FB39852F31>