> On Mar 29, 2019, at 8:09 PM, Rodney W. Grimes <[email protected]> 
> 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 in the
>>  future.
> 
> :-)  Been there, almost did that too.
> Pre commit last minute svn diff saved me.

…

This is why I’m doing the following from here on out:

$ arc patch
$ svn ci

Unfortunately svn doesn’t support all of the niceties of “arc land”. Otherwise, 
I would have used that.

The Facebook version of “arc land” (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’t 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

_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to