Details
- Reviewers
debdrup pauamma_chezdork.com
Diff Detail
- Repository
- R9 FreeBSD doc repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 59448 Build 56335: 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 | ||
|---|---|---|
| 585–588 | One sentence per line. | |
| 599–600 | One sentence per line. | |
| 606–607 | One sentence per line. | |
| 636 | I feel like this line is a bit awkward, so I've suggested something else. | |
| 685 | Errant period. | |
| 695–696 | One sentence per line. | |
| 718 | This will give incorrect sizes on filesystems with inline compression, which include ZFS on a default FreeBSD install, so I'd recommend using -A. | |
| 740–741 | One sentence per line. | |
| 853–884 | The FreeBSD handbook already has a section on dealing with Xorg (and Wayland), so I think this should refer to that instead. | |
| 903 | Errant period. | |
| 923 | 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 | ||
|---|---|---|
| 764 | Errant period. | |
| 764 | Errant period. | |
| 774 | Errant period. | |
| 787 | Capitalization. | |
| 967 | Errant period. | |
| 972 | This feels like a dangling sentence, can it be worked into the line above somehow? | |
| 1031 | Errant period and space. | |
| 1076 | This shouldn't be on a newline, since it belongs after the colon above. | |
| 1080 | Errant period. | |
| 1111 | 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. | |
| 1129 | Errant period. | |
| 1153 | Errant period. | |
| 1161 | 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. | |
| 1215 | Errant period. | |
| 1234 | Errant period. | |
| 1263 | Errant period. | |
| 1300 | Errant period. | |
| 1325 | Errant period. | |
| 1333 | 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 | ||
|---|---|---|
| 685 | 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. | |
| 853–884 | 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. | |
| 903 | 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. | |
| 923 | 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 | ||
|---|---|---|
| 680 | 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.