Skip to content
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

Better MsgId format #1566

Merged
merged 2 commits into from
Mar 15, 2023
Merged

Better MsgId format #1566

merged 2 commits into from
Mar 15, 2023

Conversation

emilk
Copy link
Member

@emilk emilk commented Mar 11, 2023

You can also switch Debug to only show the last 32 bits or so if you prefer @teh-cmc

Checklist

@emilk emilk added the 🧑‍💻 dev experience developer experience (excluding CI) label Mar 12, 2023
@Wumpf Wumpf self-requested a review March 13, 2023 15:17
@teh-cmc
Copy link
Member

teh-cmc commented Mar 13, 2023

You can also switch Debug to only show the last 32 bits or so if you prefer

Not sure I follow, looking at the Display and Debug impls in this PR, they seem to be exactly the same?

Closes #1524

#1524 is specifically about the representation used when the MsgIds are displayed in arrow/dataframe contexts (which is defined by the re_format crate iirc), which this PR doesn't address at the moment.

impl std::fmt::Display for MsgId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{:x}", self.0.as_u128())
write!(f, "{:032X}", self.0.as_u128())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also switch Debug to only show the last 32 bits or so if you prefer

This now shows the same on both though. It shows the whole thing for both Debug & Display.

@emilk
Copy link
Member Author

emilk commented Mar 13, 2023

Yeah, I naively thought the store used the Debug representation.

Anyway, this is an improvement imho

@emilk emilk merged commit c62aa91 into main Mar 15, 2023
@emilk emilk deleted the emilk/better-msg-id-format branch March 15, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants