[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Add --dry-run flag to "svn update" client command

From: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 14 Mar 2011 17:54:53 +0100

On Mon, Mar 14, 2011 at 06:34:15PM +0200, Daniel Shahaf wrote:
> Kamesh Jayachandran wrote on Fri, Mar 11, 2011 at 18:12:21 +0530:
> > Unless there are no objection I will commit this patch post I
> > receive a r1075802 regression fix(and testcase) from either Daniel
> > Becroft or Arwin.
> Hold on, are you suggesting to commit the two thousand, six hundred and
> sixty-three line diff that started this thread?

So what if it's long? I'd be much more worried if this diff was shorter.
It needs to make changes in lots of places.

It does a lot of things, including running every update during tests
with --dry-run, and checking whether the dry-run modified the working
copy, and making the test fail if it did.
And it checks if the dry-run output matches the output for normal update.
If all the tests pass with this, that's fine, isn't it?
Because how else would you do it? Not test it, just to make the diff shorter?

And I don't agree that adding a separate editor for this is going to
make things simpler. We're going to transition to editorv2 at some point.
Let's try not to make that transition more difficult by adding yet
another editor.

From my point of view, we can either add this feature and apply
this diff, or not have the feature. I'd say we should go for it,
unless we decide that we really don't want this feature.
But rejecting this diff based on line counts seems silly.

The only minor thing I noticed scrolling through the diff was that
the new dry_run paramter of run_and_verify_update isn't documented
in the docstring. That can be fixed later.
Received on 2011-03-14 17:55:39 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.