Mousius commented on pull request #9331: URL: https://github.com/apache/tvm/pull/9331#issuecomment-966162931
> Generally these assertions are derived from assumptions that a user flow will never trigger them and usually are about inputs to a particularly component. I would question why we're adding them if they're in unreachable code paths, but if we do think they're worth adding I would consider them part of the contract of the internal function and thus should have associated tests for the behaviour (throwing an exception is now part of the functions behaviour). As per [TVM Code Review Guidelines](https://github.com/apache/tvm/blob/main/docs/contribute/code_review.rst#factors-to-consider-about-code-quality), we should add test coverage for anything we contribute. If we're happy that at some point someone may inadvertently change the behaviour of this function, then potentially it's not a necessary line of code and we can apply [the YAGNI principle from extreme programming](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it) :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]
