You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/09/14 23:23:21 UTC

[GitHub] [tvm] Mousius commented on pull request #8974: Remove memory planing from LowerTEPass

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: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org