Vladimir Kotal wrote:
> <snip>
>
> This one still needs code reviewers. Again, anyone bored or with
> ksh/webrev mindset can take a look.
>
> Thanks,
>
>
> v.
>
>> webrev is here:
>> http://cr.opensolaris.org/~vkotal/webrev_delete-6771203.onnv/
The date in the .TH section of the manpage should be updated.
webrev.sh:
171, 236: When do you ever call this inappropriately? This message
doesn't seem particularly useful to the user, what does it really mean
that they need to change?
241-243: I'm not 100% confident that these are what you want. If I read
them correctly. Can't a remote_target specification be of the form
protocol://[usern...@]host.spec[:port]/
?
Specific questions:
- Do the host_spec and dir_spec assignments need to account for the
optional "username@" portion? Or does the stripped_target assignment
need to also strip that?
- I can believe that you won't see multiple occurrences of "://" in the
remote_target spec, so ## seems appropriate, but what about ":"
characters? It seems like the correct complement to "%%:*" would be
"#*:" instead of "##*:" ?
Of course, if we're always talking remote_host:remote_path, then what's
here makes sense, as long as ":" is not a legal character?
252-259: You're assuming two things here, and I think only one of them
is correct. (1) The remote_target is the path to the top level
directory of the webrev to be removed (this is valid). (2) The top
level directory of the webrev to be removed lives at the top of the
default directory tree for sftp (this is invalid).
You might avoid the second assumption by adding a "cd" command to the
batch file used by sftp? In any event, the following two remote_target
specifications should BOTH work correctly for user mjnelson:
cr.opensolaris.org:webrev.481
cr.opensolaris.org:toolsreview/webrev.bugs
...because they're both real paths.
273: So we're really collecting "deleted" webrevs forever? Or is there
something special about a ".trash" directory that isn't mentioned in the
sftp manpage? This seems evil: isn't part of the goal here to maintain
a smaller footprint? How do users REALLY get rid of them, or even known
that they SHOULD?
273: Does rename fail if the target name already exists?
280: Is this useful to the user?
2475: s/implicates/implies/
