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

Update zfs-snapshot.8 to fix a small inaccuracy in the description of snapshot atomicity #15857

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

shawnbayern
Copy link
Contributor

Fixes a small inaccuracy in the description of snapshot atomicity

zfs-snapshot(8) appears to contain a small error. The existing version reads "Snapshots are taken atomically, so that all snapshots correspond to the same moment in time." Per zfs_main.c, which in do_snapshot() simply loops over argv, this does not appear to be correct when multiple snapshots are specified explicitly on the command line. I believe the intent of the man page was to say that recursive snapshots are all created atomically.

This proposed change fixes that error. Because the existing statement may confuse some readers anyway, the commit also also adds a small amount of general explanatory information that may be helpful.

The change also adds an introductory sentence that summarizes what 'zfs snapshot' does in the first place. In that sentence, the text "different datasets" is intended to indicate that (again per the code) the same dataset cannot be specified multiple times on the command line.

Types of changes

  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf added Type: Documentation Indicates a requested change to the documentation Status: Code Review Needed Ready for review and testing labels Feb 6, 2024
man/man8/zfs-snapshot.8 Outdated Show resolved Hide resolved
man/man8/zfs-snapshot.8 Outdated Show resolved Hide resolved
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Feb 7, 2024
@shawnbayern
Copy link
Contributor Author

I noticed that the automated check flagged another minor formatting problem, so I've committed a fix.

(I'm a law professor and thought the rules for formatting legal citations were strict, but by comparison they're not! :) )

@behlendorf
Copy link
Contributor

@shawnbayern thanks!

@shawnbayern
Copy link
Contributor Author

Sorry to waste your time, but checkstyle now seems to be failing because my recent commit messages were over 72 characters per line. (I knew to avoid that from the contributors' guide and avoided it in my initial writeup, but I think my last two commit messages were over that limit - sorry!) I'm not sure of the cleanest way to fix that on GitHub to satisfy the checkstyle script; should I produce a new branch? Thanks again for your help.

@behlendorf
Copy link
Contributor

@shawnbayern no problem. You'll want to squash your commits and then force up the PR. This will kick of a new CI run. Something like this:

git checkout patch-1
git rebase -i master
# Do the squash
git push --force shawnbayern patch-1

@shawnbayern
Copy link
Contributor Author

@behlendorf thanks again! I'll keep an eye out for any further problems.

Fixes a small inaccuracy in the description of snapshot
atomicity

zfs-snapshot(8) appears to contain a small error.  The existing
version reads "Snapshots are taken atomically, so that all
snapshots correspond to the same moment in time."  Per
zfs_main.c, which in do_snapshot() simply loops over argv, this
does not appear to be correct when multiple snapshots are
specified explicitly on the command line.  I believe the intent
of the man page was to say that *recursive* snapshots are all
created atomically.

This proposed change fixes that error.  Because the existing
statement may confuse some readers anyway, the commit also also
adds a small amount of general explanatory information that may
be helpful.

The change also adds an introductory sentence that summarizes
what 'zfs snapshot' does in the first place.  In that sentence,
the text "different datasets" is intended to indicate that
(again per the code) the same dataset cannot be specified
multiple times on the command line.

Signed-off-by: Shawn Bayern <[email protected]>

Added newlines for sentences and formatting for -r flag

I've made the formatting changes suggested by Brian Behlendorf
to pass the checkstyle tests and add formatting for an inline
flag.

Signed-off-by: Shawn Bayern <[email protected]>

Removed trailing whitespace to satisfy automated checks

I removed a whitespace at the end of line 55 to satisfy the
process that verifies the style of man pages.

Signed-off-by: Shawn Bayern <[email protected]>
@behlendorf behlendorf merged commit d0d2733 into openzfs:master Feb 8, 2024
22 of 23 checks passed
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
Fixes a small inaccuracy in the description of snapshot
atomicity

zfs-snapshot(8) appears to contain a small error.  The existing
version reads "Snapshots are taken atomically, so that all
snapshots correspond to the same moment in time."  Per
zfs_main.c, which in do_snapshot() simply loops over argv, this
does not appear to be correct when multiple snapshots are
specified explicitly on the command line.  I believe the intent
of the man page was to say that *recursive* snapshots are all
created atomically.

This proposed change fixes that error.  Because the existing
statement may confuse some readers anyway, the commit also also
adds a small amount of general explanatory information that may
be helpful.

The change also adds an introductory sentence that summarizes
what 'zfs snapshot' does in the first place.  In that sentence,
the text "different datasets" is intended to indicate that
(again per the code) the same dataset cannot be specified
multiple times on the command line.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Shawn Bayern <[email protected]>
Closes openzfs#15857
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
Fixes a small inaccuracy in the description of snapshot
atomicity

zfs-snapshot(8) appears to contain a small error.  The existing
version reads "Snapshots are taken atomically, so that all
snapshots correspond to the same moment in time."  Per
zfs_main.c, which in do_snapshot() simply loops over argv, this
does not appear to be correct when multiple snapshots are
specified explicitly on the command line.  I believe the intent
of the man page was to say that *recursive* snapshots are all
created atomically.

This proposed change fixes that error.  Because the existing
statement may confuse some readers anyway, the commit also also
adds a small amount of general explanatory information that may
be helpful.

The change also adds an introductory sentence that summarizes
what 'zfs snapshot' does in the first place.  In that sentence,
the text "different datasets" is intended to indicate that
(again per the code) the same dataset cannot be specified
multiple times on the command line.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Shawn Bayern <[email protected]>
Closes openzfs#15857
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
Fixes a small inaccuracy in the description of snapshot
atomicity

zfs-snapshot(8) appears to contain a small error.  The existing
version reads "Snapshots are taken atomically, so that all
snapshots correspond to the same moment in time."  Per
zfs_main.c, which in do_snapshot() simply loops over argv, this
does not appear to be correct when multiple snapshots are
specified explicitly on the command line.  I believe the intent
of the man page was to say that *recursive* snapshots are all
created atomically.

This proposed change fixes that error.  Because the existing
statement may confuse some readers anyway, the commit also also
adds a small amount of general explanatory information that may
be helpful.

The change also adds an introductory sentence that summarizes
what 'zfs snapshot' does in the first place.  In that sentence,
the text "different datasets" is intended to indicate that
(again per the code) the same dataset cannot be specified
multiple times on the command line.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Shawn Bayern <[email protected]>
Closes openzfs#15857
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.

2 participants