On Fri, 2011-01-14 at 17:50 +0530, Noorul Islam K M wrote:
> Stefan Sperling <stsp_at_elego.de> writes:
>
> > On Thu, Jan 13, 2011 at 03:31:11PM +0530, Noorul Islam K M wrote:
> >
> >> | Command | Return Code | Continue |
> >> |------------+-------------+----------|
> >> | add | 0 | Y |
> >> | blame | 1 | N |
> >> | cat | 1 | Y |
> >> | changelist | 0 | N |
> >> | commit | 1 | N |
> >> | copy | 1 | N |
> >> | delete | 1 | N |
> >> | diff | 1 | N |
> >> | info | 1 | Y |
> >> | list | 1 | N |
> >> | log | 1 | N |
> >> | revert | 0 | Y |
> >
> > Nice overview, thanks for putting this together!
Yes, good. This shows operations on local (working copy) targets only,
doesn't it, apart from "commit". Note Stefan's observation that the
behaviour should (and probably does) vary depending on whether it's a
local operation or a repository operation, so it might be useful to
extend the table to show both.
> >> The commands add, cat, info and revert continues execution even if it
> >> finds one of the targets non-existing. All of them returns 0 to the
> >> shell except 'info' which returns 1. Also info prints
> >>
> >> svn: A problem occurred; see other errors for details
> >>
> >> at the end of the execution which tells the user that something went
> >> wrong during execution. This is not the case with 'add', 'cat' and
> >> 'revert'. I think these three commands also should return 1 and print
> >> the message which 'info' does, so that users does not have to scroll
> >> up to see the warnings to decide something went wrong.
> >
> > +1 on making more commands print a "problem occured" message at the end.
Yes, +1. (Spelling: "occurred".)
> > I also think that all commands should exit 1 if one of the specified
> > targets could not be found. This makes it much easier to detect such
> > errors in shell scripts.
> >
> >> Among these revert prints 'Skipped' message and others print 'svn:
> >> warning: ..' message.
> >
> > Hmm... somewhat inconsistent, but I don't think we need consistency
> > here. Either way of putting it ("skipped" or "warning") is fine.
I agree this is not so important. My preference is for a warning that
says what the problem was; a simple "Skipped" message is too vague for
my liking.
> >> I think commit operation is an atomic one, therefore it does make
> >> sense when it errors out when one of the target is invalid.
> >>
> >> I am not sure why all the other commands errors out when one of the
> >> targets is non-existing.
> >
> > I think any command that tries to make a commit should error out.
> > This includes copy and delete, because they can make commits without
> > a working copy.
> >
> > Commands that operate on the working copy (including copy and delete!)
> > should continue, print a "warning" or "skipped" message, and print
> > the "problem occured" message at the end.
Sounds good. So is this what we want?:
(1) If a command is making a commit (or, let's say, changing the
repository in any way, so revprop changes are included), then as soon as
an invalid target is detected, error out (print an error message and
exit with return code = 1).
(2) If a command is changing (only) the WC, and a target is invalid,
then print a warning (or "skipped" message) and move on to the next
target, and then at the end print a summary message and exit with return
code = 1.
(3) If a command is not changing the repos or WC, but only fetching
information (cat, blame, export, info, list, pg, pl, ...) then ... same
as (2)?
> If everyone agrees to this then I will start sending patches for this.
Noorul, what would the table(s) of results look like after all these
changes?
- Julian
Received on 2011-01-14 18:21:08 CET