tqchen edited a comment on pull request #7630:
URL: https://github.com/apache/tvm/pull/7630#issuecomment-802294684
Thanks @tkonolige . The level of documentation can be debatabe, while it is
always good to add more details in the docs, we also cannot too hung up on
things, to ensure timely delivery of the item, assuming the code quality is
good and readable (in this case get other committers to read and understands
the code logic).
Which such cases happens, we could apply the following principle - reviewers
should strive to read and understand the code, and suggest comments that can be
directly act upon to make sure merge happen in a reasonable pace. The
merger/committer takes the responsibility in case future problem occurs to the
case.
It is of course great to help further suggest improvements. Normally in such
cases it would be great if reviewer can make suggestions that also facilitate
merging in a quick way . I think it would be great to make comment that are
directly actionable(e.g. suggest changes that can be directly applied to the
code). This is particularly useful to help the code review move forward. Of
course doing so would demands more time from the reviewer (to read and
understand), but that is what we should strive to do in code reviews.
Here is an example of actionable comment, please feel free to suggest more.
### Example Actionable Comment
Please add the following comment to enter_block:
```
Note
------
This function is used for normal scopes that does not
involve a `with block` scope. Use `enter_block_scope`
for the other case.
```
Please add the following clarifications to `enter_block_scope`'s docstring
```
Note
------
This function should be used for the block scope,
aka the blocks that involves a `with block` scope.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
