-
Notifications
You must be signed in to change notification settings - Fork 419
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
started the burn developer book #1184
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1184 /- ##
==========================================
- Coverage 84.42% 84.37% -0.05%
==========================================
Files 549 550 1
Lines 61921 62198 277
==========================================
Hits 52278 52482 204
- Misses 9643 9716 73 ☔ View full report in Codecov by Sentry. |
Quick feedback re formatting (haven't read the content in details yet). I recommend you format the text using something like prettier if you have a VS/Neovim plug-in. We have a configuration for prettier: https://github.com/tracel-ai/burn/blob/main/burn-book/.prettierrc.json (or you can define yours) This helps formatting tables and wrapping markdown text. |
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.
These are just some general comments since this is still a work in progress.
I've commented on some typos, but since it's a draft, maybe you haven't checked them all, there are more to fix. I suggest adopting a more objective tone overall, but the content is good!
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
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.
Thank you for starting this contributor book.
Once, you're completed and merge it, I'll add a couple of section as well:
- https://github.com/tracel-ai/burn/blob/main/burn-import/DEVELOPMENT.md
- import from other formats.
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 did a first pass on the beginning of the PR with some recommendations and some typo fixes, I will do a complete proof-read for typos later.
That's an awesome PR. 👍
contributor-book/src/frequently-encountered-issues/issues-while-adding-ops.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Sylvain Benner <[email protected]>
Co-authored-by: Sylvain Benner <[email protected]>
Co-authored-by: Sylvain Benner <[email protected]>
Co-authored-by: Sylvain Benner <[email protected]>
Co-authored-by: Sylvain Benner <[email protected]>
…t.md Co-authored-by: Sylvain Benner <[email protected]>
Co-authored-by: Sylvain Benner <[email protected]>
I re-requested reviews again for approvals. I'll check it now again. One outstanding is to fix the conflicting file. |
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
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. Thanks for starting and filling up this book!
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.
Made an entire pass to essentially fix typos.
contributor-book/src/frequently-encountered-issues/issues-while-adding-ops.md
Outdated
Show resolved
Hide resolved
contributor-book/src/frequently-encountered-issues/issues-while-adding-ops.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/configuring-your-editor.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Sylvain Benner <[email protected]>
contributor-book/src/getting-started/configuring-your-editor.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
contributor-book/src/getting-started/setting-up-the-environment.md
Outdated
Show resolved
Hide resolved
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.
Awesome work !
Thanks for fixing the path to the Contributor book in the xtask command 😊
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.
👏
There a URL for it yet? |
Pull Request Template
Checklist
run-checks all
script has been executed.Related Issues/PRs
None
Changes
Adds a book that is geared toward contributors to burn. I haven't moved any of the existing content yet (such as
Architecture.md
,burn-import/contributing.md
) as I'm not sure what files should me moved or just copied/modified.Testing
I ran
mdbook build --open
locally to confirm that the book does in fact build.I read it, to confirm it is indeed readable
jokes aside, if we keep the style guidelines, should we try to run any checks to see if linked code is still exists or the current information is still up to date with main?