-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Formalize TVM Documentation Organization #27
Conversation
25d4598
to
f9a8a8f
Compare
f9a8a8f
to
9666a4f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hogepodge thanks for sending this! it'll be a welcome refactor to our documentation. I just had a couple small clarifications I wanted to request--I think the spirit of everything is captured well here but I want to make sure we're as clear as possible in the descriptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these changes will be great for all users. One question: is this RFC going to be turned into a document on where to put tutorials/how-tos/reference? I think that would be useful.
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general everything looks pretty good. Thanks @hogepodge! I've left a couple suggestions on grammar and clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from my side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One point that I think is worthwhile to bring up is by this PR: apache/tvm#8893. Maybe we could also consider future extensions like that, and make sure we have a proper place for it (Relay pass examples in this case).
* Topic Guide | ||
* Developer Guide | ||
* TVM Architecture Guide | ||
* API Reference (reference) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor note: we have language reference which served as a comprehensive reference to the language specification. see https://tvm.apache.org/docs/langref/index.html, would be great to clarify where do they fit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could go either way, in the Architecture Guide or in the Reference. Since it started as a reference document, and language references are much more status, I expanded the Reference section to include API and Language. Let me know what you think.
Co-authored-by: Tristan Konolige <tristan.konolige@gmail.com>
Any further changes requested? I'd like to start driving this work forward. |
cc @areusch would be great if we can followup, we will wait for 24hours and then merge it in if there is no further comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @hogepodge !
No description provided.