Vladimir Kotal wrote:
> Hi all again,
>
> This review request is still missing code reviewers. If someone feels
> bored or even interested, please take a look:
>
> http://cr.opensolaris.org/~vkotal/webrev_upload.onnv/
>
> Thanks,
>
>
> v.
> _______________________________________________
> website-discuss mailing list
> website-discuss at opensolaris.org
I'm not a webrev expert, but here's a few nits...
usr/src/tools/scripts/webrev.1:
371 For custom remote targets, -t option allows to specify all
components:
Better: -t option allows one to provide a completely specified upload
target
Q: Does this override the use of other options (such as -o...)?
if so, maybe the options that are overridden/ignored when -t is
used could be enumerated here as well.
usr/src/tools/scripts/webrev.sh:
164 # end source directory with backslash in order to copy just
165 # directory contents, not the whole directory
166 $RSYNC -r -q $WDIR/ $dst
Nit: The "/" character is a slash, not a backslash (which is "\")
1873 function usage
The usage message doesn't show the same set of options as implemented
in the getopt loop...
2010 # more sanity checking
2011 if [[ -n $nflag && -z $Uflag ]]; then
2012 print "it does not make sense to skip webrev generation
without -U"
2013 exit 1
2014 fi
ReallySmallNit: Usually, -n means "do everything, but only tell me
what you would have done instead of actually performing the actions".
This usage of "skip this step" seems odd. If it is not too late to
change, maybe "-g" for "don't generate"...?
Otherwise, at this micro level, looks OK to me.
-John
