Hello!
My name is Adam. This year, I participate in Google Summer of Code. I wrote manual page for my API. I would be thankful if you could take a look at it.
Thanks in advance!
Details
Details
- Reviewers
cem oshogbo - Group Reviewers
manpages - Commits
- rS304910: Introduce cnv man page.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| share/man/man9/cnv.9 | ||
|---|---|---|
| 118 ↗ | (On Diff #18552) | This is an if/then sentence, which is halting when read. Swap it around and edit a bit: The concept of cookies is explained in |
| 123 ↗ | (On Diff #18552) | Needs an ending period: .Xr nv 9 . |
| 127 ↗ | (On Diff #18552) | "allows to" is not something generally said. family of functions obtains the value of the supplied cookie. |
| 128 ↗ | (On Diff #18552) | These appear to be copies of the dnvlist man page, so I would suggest waiting for that one and using that wording. |
| 163 ↗ | (On Diff #18552) | s/during/during the/ |
Comment Actions
Thank you again wblock for your comments! Manpage updated.
| share/man/man9/cnv.9 | ||
|---|---|---|
| 128 ↗ | (On Diff #18552) | I replaced this sentence with the one you wrote for dnvlist. |
| share/man/man9/cnv.9 | ||
|---|---|---|
| 128 ↗ | (On Diff #18731) | Needs a serial comma after "binaries" to separate it from "arrays": Returned strings, nvlists, descriptors, binaries, or arrays must not be modified |
| 131 ↗ | (On Diff #18731) | "in *an* error state." |
| 136 ↗ | (On Diff #18731) | Typo: |
| 140 ↗ | (On Diff #18731) | The sentence ends on the previous line, so this line should be removed. |
| 144 ↗ | (On Diff #18731) | As above, change from "nvlist_destroy function." to "nvlist_destroy.": .Fn nvlist_destroy . and delete the next line, function. |
| 146 ↗ | (On Diff #18731) | s/with/with the/ |
| 152 ↗ | (On Diff #18731) | family of functions removes an element of the supplied cookie and frees all resources (This has been said in this or the other related man page, possibly more clearly. It might be better to copy that one.) |
| 154 ↗ | (On Diff #18731) | s/If element/If an element/ |
| 155 ↗ | (On Diff #18731) | s/will be/is/ |
Comment Actions
I think you uploaded an empty or reversed diff on accident. Phabricator shows the file as deleted instead of added. :-)
Comment Actions
It seems a bit strange to refer to close(2) and free(3) from a section 9 (kernel) man page since those functions cannot be called from the kernel. The nv(9) man page already in -current seems to have the same problem. Ideally, the man page would describe the differences between the kernel and userland versions of the APIs.
| share/man/man9/cnv.9 | ||
|---|---|---|
| 127 ↗ | (On Diff #19114) | "the value associated with the given cookie" like below may be clearer. |
| 130 ↗ | (On Diff #19114) | It may be clearer to write "must not be modified by the user, since they still belong to the nvlist." |
| 131 ↗ | (On Diff #19114) | Which nvlist? |
| 136 ↗ | (On Diff #19114) | consistency: "removes" |
| 138 ↗ | (On Diff #19114) | the returned memory |
| 149 ↗ | (On Diff #19114) | What is an "element of the supplied cookie"? |
| 154 ↗ | (On Diff #19114) | This should be EXAMPLE (capitals). |
| share/man/man9/cnv.9 | ||
|---|---|---|
| 120 ↗ | (On Diff #19153) | Needs an "and" .Fn nvlist_get_parent , and |
| 143 ↗ | (On Diff #19153) | s/with the/with/ --just "with close(2)." |
| 154 ↗ | (On Diff #19153) | Please avoid "the following", and just use "this". Also, add "the" to "the cnvlist API": This example demonstrates how to deal with the cnvlist API. |
| 191 ↗ | (On Diff #19153) | These should be sorted by section number then name. So: .Xr close 2 , .Xr free 3 , .Xr nv 9 |
Comment Actions
Missing that one "and", but otherwise looks good. Thanks!
| share/man/man9/cnv.9 | ||
|---|---|---|
| 120 ↗ | (On Diff #19153) | Still needs a new line with and. |