Add missing docstrings #1882
Add missing docstrings #1882
Conversation
cburgdorf
force-pushed
the
christoph/docs/more-api-docs2
branch
from
47aa220
to
aed33dc
Compare
cburgdorf
force-pushed
the
christoph/docs/more-api-docs2
branch
4 times, most recently
from
cbeb851
to
9955e68
Compare
cburgdorf
force-pushed
the
christoph/docs/more-api-docs2
branch
from
9955e68
to
05b8985
Compare
| @@ -65,27 +65,27 @@ def __next__(self) -> int: | |||
| return STOP | |||
|
|
|||
| def peek(self) -> int: | |||
| current_pc = self.pc | |||
| current_pc = self.program_counter | |||
There was a problem hiding this comment.
I have this in a separate commit but thought I could sneak it in here. I personally had to lookup what pc stands for again and I remember I already looked it up in the past. Part of this is probably just my poor memory but I also think it aligns better with the rest of the codebase to not use two letter acronyms :)
| ) | ||
|
|
||
|
|
||
| def validate_stack_bytes(value: bytes) -> None: | ||
| if len(value) <= 32: | ||
| return | ||
| raise ValidationError( | ||
| "Invalid Stack Item: Must be either a length 32 byte " | ||
| f"string or a 256 bit integer. Got {value!r}" | ||
| "Invalid Stack Item: Must be either a length 32 byte string. Got {value!r}" |
There was a problem hiding this comment.
Another non-doc change that I sneaked in here. I think the text was wrong.
| @@ -473,7 +587,7 @@ def add_transaction(self, | |||
| def get_block_transactions( | |||
| self, | |||
| block_header: BlockHeaderAPI, | |||
| transaction_class: Type[SignedTransactionAPI]) -> Sequence[SignedTransactionAPI]: | |||
| transaction_class: Type[SignedTransactionAPI]) -> Tuple[SignedTransactionAPI, ...]: | |||
There was a problem hiding this comment.
Another non-doc change sneaked in here. It seems more aligned with our best practices to have the return type be the actual type that is returned (in contrast to parameter types which should accept the broadest type that the method could work with). This also aligns the method with the other methods on that class that do also use Tuple[Something, ...] as return type.
| @@ -46,31 +46,31 @@ def __getitem__(self, i: int) -> int: | |||
|
|
|||
| def __iter__(self) -> Iterator[int]: | |||
| # a very performance-sensitive method | |||
| pc = self.pc | |||
| pc = self.program_counter | |||
There was a problem hiding this comment.
I would extract this to it's own PR. 👍
There was a problem hiding this comment.
Ooops..too late. I saw the approval and rage merged
What was wrong?
We have a bunch of types in our docs that are lacking docstrings. I believe having at least some docstring per public API is the lowest bar of requirement before we can cut a beta.
How was it fixed?
Added at least some docstring for every single public API.
Imperative present tense is being used for the docstrings.
For methods, it should rhyme with IT WILL:
Example:
For methods, it should rhyme with IT IS:
Example:
To-Do
Cute Animal Picture