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

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

[Yuri Pankov]

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

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

[Robert Mustacchi]

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:

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.

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).

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

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

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

Robert

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

[Yuri Pankov]

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!

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

[Robert Mustacchi]

On 6/28/16 7:58 , Yuri Pankov wrote:
> 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/

For what it's worth, it wasn't just the example shuffling, but also the
reordering and the options changing a bunch. I've tried to go through
this as best as I can.

I don't quite get why -h is going away, especially when this is noted as
committed.

Would it be possible to make sure that we have references to the
examples. e.g. see example n.

sharemgr unset used to take a [-s sharepath] option in the long form. It
doesn't seem to in the synopsis or otherwise anymore in the new form;
however, you still reference the -s option in the description. What
should it be?

Robert

[developer] share_smb(8)? (Was: 5713, 5714, 5715 updates to share man pages)

[Gordon Ross]

Hi Yuri (and developers)

I went looking for a share_smb(8) man page today, thinking "Didn't
someone create that man page?".
After some searching, I found these issues:

https://www.illumos.org/issues/5713
https://www.illumos.org/issues/5714
https://www.illumos.org/issues/5715

I then found this email thread, mentioning these webrevs:

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

Yuri, are you still around on these lists?

Anyone happen to know where I might find this work?
BTW, I'n sorry that work got stuck.  I guess people ran out of time.

Thanks,
Gordon

On Sun, Jun 19, 2016 at 10:38 AM Yuri Pankov <yuri.pankov@nexenta.com> 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
>
>
> -------------------------------------------
> illumos-developer
> Archives: https://www.listbox.com/member/archive/182179/=now
> RSS Feed: https://www.listbox.com/member/archive/rss/182179/21175075-7ec767c8
> Modify Your Subscription: https://www.listbox.com/member/?member_id=21175075&id_secret=21175075-4a7cd1de
> Powered by Listbox: http://www.listbox.com

------------------------------------------
illumos: illumos-developer
Permalink: https://illumos.topicbox.com/groups/developer/T15a7f67c761ced49-M84542f9d7d75e65b1b729d93
Delivery options: https://illumos.topicbox.com/groups/developer/subscription