-
Notifications
You must be signed in to change notification settings - Fork 528
Removes remnants of indicators from dataframe queries and viewer #10584
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
Conversation
Latest documentation preview deployed successfully.
Note: This comment is updated whenever you push a commit. |
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
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.
Had a cursory look, didn't see anything shocking. Thanks for pulling that off
pub fn header_cell_margin(&self, _table_style: TableStyle) -> Margin { | ||
Margin::symmetric(8, 6) | ||
} | ||
|
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.
Is this an easter egg to test the reviewer? :)
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.
Ahaha, nah just a bad rebase. The diff should be much nicer now that #10581 is merged.
06bfb82
to
346f250
Compare
346f250
to
923696f
Compare
@rerun-bot full-check |
Started a full build: https://github.com/rerun-io/rerun/actions/runs/16205572522 |
Related
split_indicators
fromChunkBatcher
#10583rerun_py
, andrerun_cpp
#10581What
This removes the remaining reference to indicators from our codebase. Most notably this removes indicators from the dataframe queries and the UI.
This should be fine because:
Important
There is some risk that this PR will make indicators appear again, so we should choose a good time to merge this PR. If they show up again then let's fix the root cause (i.e. migration logic).
TODO