Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize zfs(8) man page into sections #9559

Merged
merged 7 commits into from Nov 12, 2019

Conversation

overhacked
Copy link
Contributor

@overhacked overhacked commented Nov 7, 2019

Motivation and Context

As more subcommands have been added to zfs, the manpage has gotten to long to be easy to scroll through, especially on a text console. This PR is the completion of work started at the 2019 ZFS Dev Summit Hackathon. Parallel PR for the zpool command is #9564.

Description

Most subcommands got their own manpages (e.g. create).
Some related commands grouped into a single manpage and
symlinks created (e.g. set, get, and inherit). I did
this when topics were either too short to warrant their
own file or so interrelated that a user would want to
refer between commands in the same file.

Corrected .Sx internal references to .Xr cross refs

lots of .Sx references from when text was all in zfs.8
needed to be changed to .Xr zfs-$SUBCOMMAND 8 cross references.

Divided subcommand list in zfs(8) into sections

Divided into related functionality. This required writing new descriptions
for some commands.

How Has This Been Tested?

All altered manpages have been checked with mandoc -Tlint and return no warnings or errors.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

Most subcommands got their own manpages (e.g. create).
Some related commands grouped into a single manpage and
symlinks created (e.g. set, get, and inherit). I did
this when topics were either too short to warrant their
own file or so interrelated that a user would want to
refer between commands in the same file.

Corrected .Sx internal references to .Xr cross refs;
lots of .Sx references from when text was all in zfs.8
needed to be changed to .Xr zfs-$SUBCOMMAND 8 cross references.

Divided subcommand list in zfs(8) into sections of
related functionality. This required writing new descriptions
for some commands.

Removed ".Os Linux":
.Os shouldn't be specified so it can be taken from
the running operating system.

Signed-off-by: Ross Williams <ross@ross-williams.net>
Copy link
Member

@ahrens ahrens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work. I love the way you grouped the subcommand manpages into sections in the new zfs.8 manpage.

man/man8/zfs.8 Outdated Show resolved Hide resolved
man/man8/zfs-mount.8 Outdated Show resolved Hide resolved
man/man8/zfsprops.8 Outdated Show resolved Hide resolved
man/man8/zfs.8 Outdated Show resolved Hide resolved
man/man8/zfs.8 Outdated Show resolved Hide resolved
@@ -5354,7 +685,8 @@ See the
of the
.Xr smb.conf 5
man page for all configuration options in case you need to modify any options
to the share afterwards. Do note that any changes done with the
to the share afterwards.
Do note that any changes done with the
.Xr net 8
command will be undone if the share is ever unshared (such as at a reboot etc).
.El
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the SEE ALSO section that follows, should we add all the new subcommand manpages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be clutter, as all the manpage names in the DESCRIPTION section will be hyperlinked (in HTML renderings, etc.). Is there any other special handling of the SEE ALSO section that it would benefit listing all the subcommand pages?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I do think that the SEE ALSO sections of the subcommand pages need some attention, and I'll work on a commit for that before this PR is merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I'm not aware of any programmatic handling of the "see also" section.

@ahrens
Copy link
Member

ahrens commented Nov 7, 2019

Congratulations on getting this PR opened only 24 hours after winning 2nd place at the 2019 OpenZFS DevSummit Hackathon!
hackathon winners: http://open-zfs.org/wiki/OpenZFS_Developer_Summit_2019#Hackathon
video of demo: https://www.youtube.com/watch?v=rfzffM5kbe8&feature=youtu.be&t=2943

@ahrens ahrens added the Status: Code Review Needed Ready for review and testing label Nov 7, 2019
@codecov
Copy link

codecov bot commented Nov 7, 2019

Codecov Report

Merging #9559 into master will decrease coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9559      +/-   ##
==========================================
- Coverage   79.16%   79.06%   -0.11%     
==========================================
  Files         418      418              
  Lines      123686   123686              
==========================================
- Hits        97917    97789     -128     
- Misses      25769    25897     +128
Flag Coverage Δ
#kernel 79.75% <ø> (+0.06%) ⬆️
#user 66.61% <ø> (-0.45%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 734de7c...98d9307. Read the comment docs.

`.Os` macro parsing behavior differs between mandoc from
the "BSD" mandoc package (available on Ubuntu) and
man from Ubuntu's man-db package, which calls groff
to format the manpages.

Groff handles the `.Os` macro differently and wrongly,
defaulting it to "BSD" in `/usr/share/groff/*/tmac/mdoc/doc-common`,
instead of getting the default from `uname`.

A future set of changes will introduce build-time
preprocessing of manpages for platform-specific documentation
and can insert the correct operating system name.
Parameter for `zfs unmount` was too far outdented
because the .It was enclosed in the wrong .Bl/.El pair.
Some text belonging to the description of
`zfs-inherit(8)` had been shifted down a few lines.
Restructuring manpage makes it make more sense to
summarize the argument name as `subcommand`, matching
the `Subcommands` subsection.

h/t @ahrens
Moved 1 paragraph down, because the first paragraph
is really an introduction.
h/t @ahrens
@overhacked overhacked mentioned this pull request Nov 8, 2019
12 tasks
.Sy rename ,
and
.Sy share .
These permissions cannot be delegated because the Linux
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure where this particular limitation should be stated -- and we should try to have the limitations for each of the various OSes, where possible, listed. (FreeBSD can do most of those, for example.) I'm not saying this needs to change now, but you may want to decide on a different format. Alternately, we may need to have separate man pages for OS-specific limitations. Thoughts?

Copy link
Member

@ahrens ahrens Nov 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could keep this information close at hand - i.e. in the zfs-allow.8 manpage rather than having inaccurate information here and then a separate zfs-linux.8 manpage that has corrections to the other manpages.

But at some point we should definitely think about how to maintain the docs for multiple platforms from one manpage source file. That might be as simple as noting platform differences in the manpages, or we might have something like preprocessor macros to "compile" the proper manpage for each platform.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However it might at least be worth noting BSD compatibility under the OS tags already, I guess most of it should be pretty much BSD compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of the preprocessor approach with maybe some brief OS-compatiblity notes with a link to openzfs.org added to zfs.8 that appear on every platform. That said, for content additions and removals, I'd like to open another issue for discussion so that this PR can get merged with only the relocation of existing subcommand content to new files and trivial clean-up.

projectquota other Allows accessing any projectquota@... property
projectobjused other Allows reading any projectobjused@... property
projectused other Allows reading any projectused@... property

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be done using a .Bl macro with -column? It may admittedly be too verbose for it (normally I'd use tbl, but I forget whether man pages can easily use that).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that some of the content needs clean-up, but let's push these items into another issue for a future PR. I think this would fit nicely into my next docs PR project which is to add OS-specific preprocessing to the man pages. I'll put a comment on this PR when I open that issue.

@behlendorf behlendorf added the Type: Documentation Indicates a requested change to the documentation label Nov 9, 2019
@lundman
Copy link
Contributor

lundman commented Nov 10, 2019

Ah great - it had been on my mind on how we will handle the apple-only parts on the manpage, as they are frequently lost when merging with upstream.

The newly-divided zfs-*.8 subcommand man pages
needed their own SEE ALSO sections pointing to related
subcommands and, in some cases, documentation from
other packages (e.g. zfs-share.8).
Copy link
Contributor

@Ornias1993 Ornias1993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, needly structured.
Also: thumbs up for quickly implementing feedback!

@overhacked
Copy link
Contributor Author

Pending any review issues, I can say this is ready now that the SEE ALSO sections are in place.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, it really makes it easier to find what your looking for in the man pages. Thanks for taking on this work.

Locally everything formats nicely for me when reading the updated man pages. However, when running mandoc -Tlint with mandoc-1.14.4 I did see some WARNING and STYLE messages, but no ERRORs. This is probably due to me using newer version which is a bit pickier.

I'd suggest we tackle that work in a follow up PR, and as part of that work update the top-level make mancheck target to run mandoc on all of the new man pages.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 11, 2019
@overhacked
Copy link
Contributor Author

Agreed about the warnings. I'd love to address all of them so that the build process can be clean. Most are basically whitespace errors about lines not being broken at the end of sentences, I guess the way mandoc likes it. The other major class of warning seems to be about missing cross-referenced manpages, which I imagine is more of an issue between different OSes and bears further investigation. No point in linking to manpages in the wrong section or that have slightly different names; another item to address in OS-specific preprocessing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested) Type: Documentation Indicates a requested change to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants