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

Re: svn commit: rev 1055 - trunk/subversion/clients/cmdline

From: Karl Fogel <kfogel_at_newton.ch.collab.net>
Date: 2002-01-25 17:50:12 CET

Daniel Stenberg <daniel@haxx.se> writes:
> > Some comments, the main one being the lack of error handling. This is
> > particularly important for the commit code!
> Yes, and that's why I added the comment in the commit message and in my
> pre-commit report, that I intend to add error codes and checks as soon as
> there's a good way to do that in the client.

Fixing up client error codes is a separate change from this -- do one
change or the other, but please don't mix them together in a dependent
way, such that you check in code you know is wrong.

There is a *workable* system for returning and checking errors in the
client right now. If you're going to check in the $EDITOR change
first, then use that workable system to its fullest. Fixing the error
system is another task, and it would be wonderful if you want to take
it on -- but prior commits shouldn't count on your doing that!

Sorry if this comes off a bit harsh, Daniel, but it looks like you're
spending more effort on defending your change against Philip's (and
Mike's) criticisms than on absorbing what they said. Their criticisms
are right on target. IMHO, they've paid you a great complement by
giving such detailed review. Don't defend against it, learn from it!

> A few of your remarks were on code I copy-n-pasted directly from the status
> code... :-)
> Thanks anyway.

If so, the presence of buggy code elsewhere in Subversion doesn't mean
we have to propagate it. :-) If you disagree with something Philip
said, defend your code on its merits, not on where you got it from.


To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:59 2006

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