Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Mar 2019 22:10:10 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        Garrett Cooper <yaneurabeya@gmail.com>
Cc:        "Rodney W. Grimes" <rgrimes@freebsd.org>, Garrett 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:  <CANCZdfpvajOreuD96=6THcRPGnCkNhJjcycTWPVWsog%2BRQeShg@mail.gmail.com>
In-Reply-To: <E790A24C-E935-45CF-A957-98FB39852F31@gmail.com>
References:  <201903300309.x2U39DtW002526@gndrsh.dnsmgr.net> <E790A24C-E935-45CF-A957-98FB39852F31@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Mar 29, 2019, 9:15 PM Enji Cooper <yaneurabeya@gmail.com> wrote:

>
> > On Mar 29, 2019, at 8:09 PM, Rodney W. Grimes <freebsd@gndrsh.dnsmgr.ne=
t>
> wrote:
> >
> >> Author: ngie
> >> Date: Fri Mar 29 18:43:46 2019
> >> New Revision: 345707
> >> URL: https://svnweb.freebsd.org/changeset/base/345707
> >>
> >> Log:
> >>  Revert r345706: the third time will be the charm
> >>
> >>  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.
> >
> > 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 :-)
> >
> >>  For my own personal sanity, I will try not to mix reviews like this i=
n
> the
> >>  future.
> >
> > :-)  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=
=9Carc 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 resub=
mit 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.
>

I use git svn and git show every commit so I'm sure when I want to do
multiple at once. Phab doesn't make that easy, alas, so I often choose the
lesser evil of just getting a review on the squashed branch. I only split
when there is a good reason. If we had better tools, I wouldn't be forced
to pick, but you go to wat with the tools you have...

Warner

>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfpvajOreuD96=6THcRPGnCkNhJjcycTWPVWsog%2BRQeShg>