Add field docstrings to public fields of stdlib entities #2588
Conversation
mfelsche
added
enhancement: 4 - in progress
do not merge
mfelsche
force-pushed
the
field-docstring
branch
from
b6a9ab2
to
6e46b2c
Compare
mfelsche
force-pushed
the
field-docstring
branch
from
6e46b2c
to
78c8f99
Compare
mfelsche
removed
do not merge
packages/builtin/env.pony
Outdated
| """ | ||
| The root capability. | ||
|
|
||
| Can be `None` for artificially constructed `Env`instances. |
There was a problem hiding this comment.
Missing a space before "instances".
packages/builtin/env.pony
Outdated
| 512) | ||
| ``` | ||
|
|
||
| **Note:** For reading user input from a terminal, use the `readline` package. |
There was a problem hiding this comment.
Maybe most of this docstring would be more appropriate moved to the file where the Stdin actor is defined?
packages/files/directory.pony
Outdated
| @@ -22,6 +22,10 @@ class Directory | |||
| Capsicum. | |||
| """ | |||
| let path: FilePath | |||
| """ | |||
| The filesystem path pointing to this directory. | |||
There was a problem hiding this comment.
I think we should emphasize here that the FilePath type is an object capability that grants access to do stuff in this directory - it's more than just a string with a path.
packages/files/file.pony
Outdated
| @@ -80,7 +80,15 @@ class File | |||
| Operations on a file. | |||
| """ | |||
| let path: FilePath | |||
| """ | |||
| The filesystem path to this file. | |||
packages/files/file.pony
Outdated
| let writeable: Bool | ||
| """ | ||
| `true` if this file is writable. |
There was a problem hiding this comment.
This one seems to not be conveying any useful information beyond that which is contained in the line above it.
Maybe it's better to say something like "the underlying file descriptor was opened as writable?
Also, should we consider fixing the misspelling of this field name now that it's been noticed?
There was a problem hiding this comment.
I took a look around and it seems, both variants are in use, writable and writeable, although writable seems to be more common. (I am not a native english speaker, so correct me if i'm wrong).
I am a bit hesitant to introduce a breaking change like this into this PR that should only be about adding documentation.
packages/files/file_info.pony
Outdated
| @@ -15,21 +9,79 @@ class val FileInfo | |||
| let filepath: FilePath | |||
|
|
|||
| let mode: FileMode val = recover FileMode end | |||
| """UNIX-style file mode""" | |||
There was a problem hiding this comment.
Can we make this more consistent with some of the other field docstrings and add punctuation at the end?
packages/files/file_info.pony
Outdated
| let hard_links: U32 = 0 | ||
| """number of hardlinks to this `filepath`""" |
There was a problem hiding this comment.
Can we capitalize this one and add punctuation at the end?
packages/json/json_type.pony
Outdated
| @@ -7,6 +7,7 @@ type JsonType is (F64 | I64 | Bool | None | String | JsonArray | JsonObject) | |||
|
|
|||
| class JsonArray | |||
| var data: Array[JsonType] | |||
| """the actual array containing JSON structures""" | |||
There was a problem hiding this comment.
Can we capitalize this and add punctuation at the end?
packages/json/json_type.pony
Outdated
| @@ -73,6 +74,10 @@ class JsonArray | |||
|
|
|||
| class JsonObject | |||
| var data: Map[String, JsonType] | |||
| """ | |||
| the actual JSON object structure, | |||
There was a problem hiding this comment.
Can we capitalize the start of this sentence?
packages/net/net_address.pony
Outdated
| let addr4: U32 = 0 | ||
| """ | ||
| bits 97-128 of the IPv6 address in network byte order. |
There was a problem hiding this comment.
Can we capitalize the start of this sentence?
Same comment goes for addr1-addr3 above.
mfelsche
force-pushed
the
field-docstring
branch
from
1f31aba
to
5c659ac
Compare
where it is possible and makes sense.
With the merge of #2543 this is now syntactically valid.
Please feel free to give better suggestions for any docstring you think is wrong, inappropriate or incomplete.