Relax SorbetColumnDescriptors and add ChunkColumnDescriptors
#9934
Conversation
emilk
added
🚜 refactor
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/14914258965 |
|
Web viewer built successfully. If applicable, you should also test it:
Note: This comment is updated whenever you push a commit. |
|
@rerun-bot full-check |
|
Started a full build: https://github.com/rerun-io/rerun/actions/runs/14914319005 |
| impl std::ops::Deref for SorbetColumnDescriptors { | ||
| type Target = [ColumnDescriptor]; | ||
|
|
||
| #[inline] | ||
| fn deref(&self) -> &Self::Target { | ||
| &self.columns | ||
| } | ||
| } |
There was a problem hiding this comment.
In general, I dislike Deref in any other context than smart pointer (as per the documentation). In this particular case, I'm afraid in potential method name collisions down the line. I'd much rather have a as_slice() method, which happens to be rather standard in std.
There was a problem hiding this comment.
In this case I disagree - SorbetColumnDescriptors is just a Vec<ColumnDescriptor> with some helper methods on it, and I think that is a perfect use of Deref
There was a problem hiding this comment.
I'll note that there is already an ambiguity with IntoInterator, since both SorbetColumnDescriptors and its deref target implements it, but that's not a hill i'm willing to die on.
Related
What
SorbetColumnDescriptorsno longer enforces the order of columns. Instead there is a newChunkColumnDescriptorsthat enforces this.