Julian Foad <julian.f...@wandisco.com> writes:
> On Fri, 2011-01-14 at 17:50 +0530, Noorul Islam K M wrote:
>
>> Stefan Sperling <s...@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?
>
I included return code for URL related commands in the below table.
Commands add, changelist, commit and revert are WC only commands.
'delete' is a commit operation when performed on URL. So I have not
included them here in URL list.
| Command | Return Code | Continue |
|------------+-------------+----------|
| blame | 1 | N |
| cat | 1 | N |
| copy | 1 | N |
| diff | 1 | N |
| info | 1 | Y |
| list | 1 | N |
| log | 1 | N |
After all these changes the table will look like this.
For WC related commands:
I removed commit because it is an atomic one and it will error out. So
no change.
| Command | Return Code | Continue |
|------------+-------------+----------|
| add | 1 | Y |
| blame | 1 | Y |
| cat | 1 | Y |
| changelist | 1 | Y |
| delete | 1 | Y |
| diff | 1 | Y |
| info | 1 | Y |
| list | 1 | Y |
| log | 1 | Y |
| revert | 1 | Y |
For URL related commands:
| Command | Return Code | Continue |
|------------+-------------+----------|
| blame | 1 | Y |
| cat | 1 | Y |
| copy | 1 | Y |
| diff | 1 | Y |
| info | 1 | Y |
| list | 1 | Y |
| log | 1 | Y |
Let me know if you have any comments.
Thanks and Regards
Noorul
