-
Notifications
You must be signed in to change notification settings - Fork 373
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 codegen #3445
Docs codegen #3445
Conversation
### What * Fixes #2461 The new link format is `https://static.rerun.io/{original_image_name_minus_extension}/{content_hash}/{width}.{ext}` For example, `https://static.rerun.io/ae4b8970edba8480431cb71e57b8cddd9e1769c7_clock_480w.png` is now `https://static.rerun.io/clock/ae4b8970edba8480431cb71e57b8cddd9e1769c7/480w.png`. What makes this more discoverable: - For machines, it's the fact that the hash is the same for all sizes. This means given an image URL, you can retrieve all the other sizes. This is important for #3445. - For humans, the top-level directory is the name of the image, which makes it _much_ easier to browse through GCS looking for that image. You can compare the `updated at` date of each hash to find the latest one. `upload_image.py` has been updated to use the new format. We're nowhere near the point of using too much space on GCS, so all the old links are preserved. This PR also fixes some other issues: - `thumbnails.py` did not handle updating `thumbnail_dimensions` correctly if the property was already present in the frontmatter - `upload_image.py` would fail if the user does not have `delete` permission when uploading the same image twice, because it would attempt to overwrite the existing image. if the content hash does not change, then we do not need to upload the image again, so we just skip uploading to GCS if the object already exists. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3477) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3477) - [Docs preview](https://rerun.io/preview/67a25d43432ddf6245f04ca52806fd976a4dac4e/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/67a25d43432ddf6245f04ca52806fd976a4dac4e/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
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!
if let Some(title) = example.base.title { | ||
lines.push(format!(" ### {title}")); | ||
} | ||
lines.push(" ```ignore".into()); |
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.
Why ignore
? All the code examples should properly parse.
no_run
should work just as well
lines.push(" ```ignore".into()); | |
lines.push(" ```no_run".into()); |
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.
ignore
sounds correct to me.
All of these are already compiled and ran as part of the code_examples
crate; recompiling them here would only slow down the CI for very little benefit?
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.
fair; I guess we don't need the extra sanity check
@@ -58,6 +58,8 @@ | |||
from google.cloud import storage | |||
from PIL.Image import Image, Resampling | |||
|
|||
# NOTE: We depend on the stack using the exact sizes they do right now in: | |||
# - docs codegen (`image_url_stack`) |
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.
very good!
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
Co-authored-by: Emil Ernerfeldt <[email protected]>
We also need to remove the old archetype pages and update the index page as well |
I updated the index page, I see you already opened #3504 - I'll just merge this then once CI passes |
What
Closes #2775
.rs
/.py
filesFollow-up work: #3503
Checklist