On Fri, 2005-04-01 at 17:46, Julian Foad wrote:
> Madan US wrote:
> > Sending for the third time. Committers pl. comment.
>
> I saw it every time you posted it. Please leave more than a day between
> re-posts unless it is urgent. I go away for several days at a time.
>
> This patch doesn't seem very important, but I apologise for not commenting
at all.
>
oh, okay. I waited for a day and posted again... I thought it wasn't
noticed. I'll take your advice. I will wait for some more time before
repost in future. Yes, its not very important... but just making sure it
got in... :-)
> > [[[
> > Improved test for Issue #2214: poor error message when
> > intermediate dir missing in copy.
> >
> > * subversion/tests/clients/cmdline/copy_tests.py
> > (url_to_non_existent_url_path): Improved to
> > check for exact error message
> > ]]]
>
> Thank you for noticing that "improvise" is not the same as "improve",
> and for> fixing it here, though your attached log message is still the old one.
(There
> is no need to re-post just to fix that.)
>
is it? I thought I changed it.... anyways, I will make this change along
with the extra space removal thing below and send the patch again.
> What is the reason for this change? There is no requirement for "svn" to
> output this exact error message. On the other hand, your fix was to
> improve> the error message from an inappropriate one to this appropriate one, and
> for> that it might be worth having a regression test to ensure that the old
error is
> not brought back by a merge or whatever. OK, I suppose that is a
> sufficient> reason for this change.
>
> > Index: subversion/tests/clients/cmdline/copy_tests.py
> > ===================================================================
> > --- subversion/tests/clients/cmdline/copy_tests.py (revision 13736)
> > +++ subversion/tests/clients/cmdline/copy_tests.py (working copy)
> > @@ -1482,19 +1482,23 @@
> >
> > sbox.build()
> > wc_dir = sbox.wc_dir
> > -
> > - dirURL1 = svntest.main.current_repo_url + "/A/B/E"
> > - dirURL2 = svntest.main.current_repo_url + "/G/C/E/I"
> > -
> > +
>
> You inserted some long lines full of spaces here...
>
> > + dirURL1 = svntest.main.current_repo_url + "/A/B/E"
> > + dirURL2 = svntest.main.current_repo_url + "/G/C/E/I"
> > + msg = ".*: Path 'G' not present"
> > +
>
> and here.
>
My bad. Will fix that.
> > # Expect failure on 'svn cp SRC DST' where one or more ancestor
> > # directories of DST do not exist
> > - svntest.actions.run_and_verify_svn(None, None, SVNAnyOutput,
> > - 'cp', dirURL1, dirURL2,
> > - '--username',
> > svntest.main.wc_author,> > - '--password',
> > svntest.main.wc_passwd,> > - '-m', 'fooogle')
> > + out, err = svntest.main.run_svn(1,
> > + 'cp', dirURL1, dirURL2,
> > + '-m', 'marina')
>
> You have removed the username and password options but not mentioned that
> change in the log message. Why?
>
I thought the log message should capture the major reason for change.
Should we write all the changes in the logs? Pl. correct me if I'm
wrong.
And specifically, the reason for change is that it is not required. I
believe the svntest module uses a default username and password even if
not provided.
> > + for err_line in err:
> > + if re.match (msg, err_line):
> > + break
> > + else:
> > + print "Error:\n" + msg + "\nnot found in error output:\n" + str(err)
> > + raise svntest.Failure
> >
> > -
>
> - Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 2 20:31:29 2005