Re: [developer] [REVIEW] (again) 5713, 5714, 5715 updates to share man pages

[Yuri Pankov <yuri.pankov@nexenta.com>]

On Tue, 21 Jun 2016 16:50:41 -0700, Robert Mustacchi wrote:
> On 6/19/16 7:38 , Yuri Pankov wrote:
>> issue: https://www.illumos.org/issues/5713
>> issue: https://www.illumos.org/issues/5714
>> issue: https://www.illumos.org/issues/5715
>> webrev: http://www.xvoid.org/illumos/webrev/il-man-share/
>>
>> The changes are tied together thus one webrev.
>>
>> - remove NFS/SMB options and access_list description from sharemgr(1M)
>> - remove access_list description from share_nfs(1M)
>> - add share_smb(1M) describing SMB options
>> - add shareacl(4) describing access_list format
>
> I started going through this and I'm a bit confused. Your sharemgr(1M)
> in sdiffs doesn't look anything like the one currently in the gate. What
> was this built against? I have more specific comments below:

Sorry, forgot the conversion diff, which also moved the examples to 
where they belong - I understand that this makes the review harder, and 
I'm sorry for that, but I did that work long ago and just don't want to 
see it got lost.

http://www.xvoid.org/illumos/webrev/il-man-sharemgr-mdoc/

> man/man1m/sharemgr.1m:
>
> I get that you're trying to consolidate information here that used to be
> here, but you need to give the user some kidn of hint of where to look.
> You've removed all the content and there's not even a See Also. In
> general, I'd expect there to be at least something like you had in the
> share_nfs(1M) changes.

Added the man pages to SEE ALSO, and added the text to 'share' 
subcommand description.

> man/man1m/share_smb.1m: Note line numbers correspond to the raw manual page.
>
> - In general I find the description a little confusing. The mount manual
>   pages separate out the distinction between the core mount binary doing
>   things and for example, what mount_nfs does. I get that share is what
>   ultimately from a user perspective does this, but it's not that all
>   invocations of share do this. Maybe change things here to say that
>   it's share_smb that starts this in a similar fashion to mount_nfs(1M).

Updated the wording in both share_nfs(1M) and share_smb(1M).

> - What are valid strings for the cksumlist argument to the cksum option?

Thanks for asking this question, I incorrectly moved that option to 
share_smb(1M) man page, while it should have gone to share_nfs(1M), but 
looks like it isn't implemented in libshare at all, so simply removed.

> +40: This should be .Sh, not .Ss in our manual page conventions.

Fixed. I wasn't sure about this either, most likely we need to add it to 
mdoc(5), or have a separate man page describing what *our* man pages 
should look like, mdoc(5) shouldn't really describe that and do only markup.

> +196, +202, +206: Presumably these should all include the reference that
> the format is in shareacl(4) like the share_nfs page does.

Done.

incremental webrev: 
http://www.xvoid.org/illumos/webrev/il-man-share-review1/

Note that I fixed the width specifiers in share_nfs(1M), the *real* 
changes are at the top.

And thanks for review, Robert!