-
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
Add "rerun_example_" prefix to all our user-visible app-ids #3112
Conversation
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 we need to decide on how to format the recording id strings and then do it the same everywhere. This looks way too sloppy as one of the first touch points a user gets of Rerun.
I prefer "Rerun example: annotation context rects" but we could also go with "rerun-example-annotation-context-rects" or "rerun_example_annotation_context_rect", as long as we're consistent.
docs/code-examples/depth_image_3d.py
Outdated
@@ -9,7 +9,7 @@ | |||
image[130:180, 100:280] = 45000 | |||
|
|||
|
|||
rr.init("depth_image", spawn=True) | |||
rr.init("rerun-example-depth_image", spawn=True) |
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 all these actually look quite ugly. I'd honestly prefer if we just used regular whitespace for all the recording id's. What's even the reason for using snake or kebab case?
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.
With "these" I'm particularly referring to the snake_-kebab
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.
The reason is that this is an id, not a name or title.
A user reading "Rerun example: annotation context rects"
is likely to interpret that string as a title, and would probably be surprised that changing it slightly would have side-effects, like Rerun no longer associating a previously stored blueprint with the new recording.
I switched all app-ids to |
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.
👍
Size changes
|
What
The exact prefix is not super important. Once there, we can search-replace easily.
I also added the
app_id_starts_with_rerun_example
analytics signal for this (needed since we hash the app id).The old
is_official_example
is still there (its not implemented for C++ though).Review commit-by-commit
Checklist