Mousius commented on pull request #9331:
URL: https://github.com/apache/tvm/pull/9331#issuecomment-966193782


   > **If it means we need a add a borderline meaningful test that uses as an 
assertion to see if the assertion inside the code is triggered, lets do that 
then**.
   
   It doesn't, you're the committer responsible for this PR, if you genuinely 
disagree feel free to merge without test coverage - this has happened on other 
PRs I've reviewed.
   
   > I guess what you are saying is we need a test for the above ICHECK in this 
PR?
   
   Yip, I would advocate for full test coverage in line with [the TVM Code 
Review 
Guidelines](https://github.com/apache/tvm/blob/main/docs/contribute/code_review.rst#factors-to-consider-about-code-quality),
 working on the assumption we want the code to continue working for the 
forsee-able future. Though I don't think we should turn this PR into a debate 
on software testing - that's been going for a long time now :smile_cat: .


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to