Details
- Reviewers
debdrup pauamma_gundo.com
Diff Detail
- Repository
- R9 FreeBSD doc repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 59397 Build 56284: arc lint + arc unit
Event Timeline
I did a quick first pass since I had a bit of energy remaining.
Out of curiosity, are you using textproc/igor and vale?
| documentation/content/en/books/handbook/virtualization/_index.adoc | ||
|---|---|---|
| 1292–1295 | One sentence per line. | |
| 1306–1307 | One sentence per line. | |
| 1313–1314 | One sentence per line. | |
| 1343 | I feel like this line is a bit awkward, so I've suggested something else. | |
| 1392 | Errant period. | |
| 1402–1403 | One sentence per line. | |
| 1425 | This will give incorrect sizes on filesystems with inline compression, which include ZFS on a default FreeBSD install, so I'd recommend using -A. | |
| 1447–1448 | One sentence per line. | |
| 1560–1591 | The FreeBSD handbook already has a section on dealing with Xorg (and Wayland), so I think this should refer to that instead. | |
| 1610 | Errant period. | |
| 1630 | Errant period. | |
Did a pass on the remaining bit that was added, but it's entirely possible I've missed something.
| documentation/content/en/books/handbook/virtualization/_index.adoc | ||
|---|---|---|
| 1471 | Errant period. | |
| 1471 | Errant period. | |
| 1481 | Errant period. | |
| 1494 | Capitalization. | |
| 1674 | Errant period. | |
| 1679 | This feels like a dangling sentence, can it be worked into the line above somehow? | |
| 1738 | Errant period and space. | |
| 1783 | This shouldn't be on a newline, since it belongs after the colon above. | |
| 1787 | Errant period. | |
| 1818 | This might need wordsmithing a bit, to better flow with the list above, or might need to be included in the list above? Not entirely sure. | |
| 1836 | Errant period. | |
| 1860 | Errant period. | |
| 1868 | Starting a sentence with "Then.." only works if it's as a consequence of what precedes it, so I think this needs to be wordsmithed a bit. | |
| 1922 | Errant period. | |
| 1941 | Errant period. | |
| 1970 | Errant period. | |
| 2007 | Errant period. | |
| 2032 | Errant period. | |
| 2040 | Errant period. | |
It seems that my previous replies have not been submitted, though I don't know why.
Anyway, here is a consolidated look at your comments.
All of the "Errant period" comments fall into one of two categories:
- Ordered lists - these are the "period at beginning of line followed by space" variety. Ordered lists are covered in the FDP Primer, section 6.4.1
- Image captions - these are the "period at beginning of line followed by text" variety. Image captions are covered in the FDP Primer in section 7.1 under the "Images" row in the table.
I will mark all of those comments "Done".
Regarding
- "Capitalization" at line 704 - vale insisted on lower case, so I used lower case, though it didn't seem right to me. I will change it back to "ZFS".
- Comments at 889, 993, and 1028 I will fix.
- At 1078, the sentence starting with "Then..." is the middle sentence of a series of three sentences in the text. Seems like it fits well to me. Let me know if you agree. Happy to change. Take a look at the online version at:
Thanks for your review. I will post another update tomorrow.
| documentation/content/en/books/handbook/virtualization/_index.adoc | ||
|---|---|---|
| 1392 | I don't see an errant period. The first period is for continuing the numbered list. The second period is part of the [.filename] asciidoc sequence. The third period is the end of the sentence. | |
| 1560–1591 | The Handbook section for Xorg is delves into the minutiae of video cards and xorg.conf details which are not relevant here. There is also no discussion of setting up a desktop manager such as XFCE. The section as written has the minimum detail needed to get a functional desktop and punts to the XFCE site for usage. | |
| 1610 | This construction is used as the caption for the images. Removing the period causes the image to disappear. Removing the entire line causes the caption to disappear. | |
| 1630 | Used for the image caption. See previous comment. | |
Aah, that makes sense, I'd missed/forgotten that.
Regarding
- "Capitalization" at line 704 - vale insisted on lower case, so I used lower case, though it didn't seem right to me. I will change it back to "ZFS".
- Comments at 889, 993, and 1028 I will fix.
- At 1078, the sentence starting with "Then..." is the middle sentence of a series of three sentences in the text. Seems like it fits well to me. Let me know if you agree. Happy to change. Take a look at the online version at:
Fair enough, I guess it makes sense - it's just that in the source code, it stands out more because of the one sentence per line thing that AsciiDoc uses.
Just one minor nit, fixable on commit.
| documentation/content/en/books/handbook/virtualization/_index.adoc | ||
|---|---|---|
| 679 | Nit while here. | |
This revision has somehow gotten out of sync with Phabricator and git arc. Abandoning this revision and will create a new one.