Details
- Reviewers
debdrup pauamma_gundo.com
Diff Detail
- Repository
- R9 FreeBSD doc repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 59393 Build 56280: 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 | ||
|---|---|---|
| 502–505 | One sentence per line. | |
| 516–517 | One sentence per line. | |
| 523–524 | One sentence per line. | |
| 553 | I feel like this line is a bit awkward, so I've suggested something else. | |
| 602 | Errant period. | |
| 612–613 | One sentence per line. | |
| 635 | This will give incorrect sizes on filesystems with inline compression, which include ZFS on a default FreeBSD install, so I'd recommend using -A. | |
| 657–658 | One sentence per line. | |
| 770–801 | The FreeBSD handbook already has a section on dealing with Xorg (and Wayland), so I think this should refer to that instead. | |
| 820 | Errant period. | |
| 840 | 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 | ||
|---|---|---|
| 681 | Errant period. | |
| 681 | Errant period. | |
| 691 | Errant period. | |
| 704 | Capitalization. | |
| 884 | Errant period. | |
| 889 | This feels like a dangling sentence, can it be worked into the line above somehow? | |
| 948 | Errant period and space. | |
| 993 | This shouldn't be on a newline, since it belongs after the colon above. | |
| 997 | Errant period. | |
| 1028 | 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. | |
| 1046 | Errant period. | |
| 1070 | Errant period. | |
| 1078 | 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. | |
| 1132 | Errant period. | |
| 1151 | Errant period. | |
| 1180 | Errant period. | |
| 1217 | Errant period. | |
| 1242 | Errant period. | |
| 1250 | 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 | ||
|---|---|---|
| 602 | 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. | |
| 770–801 | 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. | |
| 820 | 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. | |
| 840 | 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 | ||
|---|---|---|
| 497–1289 | 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.