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


   > I'll confess I've been doing the same in my PRs and tend to clean up 
spelling mistakes and minor style glitches as I encounter them without 
factoring them out. Absolutely agree ideally they'd go in as 'no semantic 
change' PRs, but the CI delay and review overhead makes me much more forgiving. 
That said I left two suggestions to lower the diff.
   
   > Also I second the thing about additional cleanups-- given CI time it takes 
a lot of time to pull these out.
   
   Whilst I can appreciate the annoyance that CI brings, this PR introduces 
more changes in unrelated files than I'm comfortable with. Now that @mbs-octoml 
has articulated the `tec::` and `./` changes, that seems in line with the 
purpose of this PR. However, I'd suggest the commit history should not show 
that removing memory planning was the last change to the issue templates, the 
microTVM Arduino port, Ethos U, Vitis AI and Relay docs.
   
   Also this PR seems to have inadvertently reverted 
https://github.com/apache/tvm/pull/8973?
   
   I'd propose integrating @mbs-octoml's comments and then scoping the commit 
to just `src/relay` for final review?
   
   


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