tqchen commented 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).
In this case the first principle should be -- 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 the motive would be 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.
### Example Direct 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 the code
```
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]
