-
Notifications
You must be signed in to change notification settings - Fork 394
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
Docs inline viewer infrastructure #5438
Conversation
Size changes
|
I did all the renaming in 521415f and onward (though there are some mentions of "snippet" mixed in the earlier commits). Hopefully it's a bit easier to review this way. |
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.
Looks good, except the double panic hook
It would have been easier to review if the "code-example -> snippet" renaming had come in its own PR though
|
||
if !path.extension().is_some_and(|p| p == "py") { |
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.
giving complex expressions a name can greatly improve readability
if !path.extension().is_some_and(|p| p == "py") { | |
let has_python_extension = path.extension().is_some_and(|p| p == "py"); | |
if !has_python_extension { |
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 this one is already pretty readable, putting it into a variable feels like I'm repeating myself
I just wanted to get it over with, because after this PR we would have:
All mean the same thing. |
Snippets™️ (formerly known as "code examples" or "doc examples") are now built on CI together with Examples™️, and uploaded to the same place.
Also includes some changes to example screenshots (adding
data-inline-viewer
attributes), and updating the egui rev to fix a bugrequest_animation_frame
when destroying web runner emilk/egui#4169Checklist
main
build: app.rerun.ionightly
build: app.rerun.io