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

tests: normalize hexadecimal data representation #945

Merged
merged 5 commits into from
Nov 4, 2022

Conversation

jmillan
Copy link
Member

@jmillan jmillan commented Nov 4, 2022

Each byte is represented as 0x%02x separated with comma and space within each other.

We were mixing many styles: 0x, b, d...

Each byte is represented as 0x%02x separated with comma and space within
each other.
@jmillan jmillan requested a review from ibc November 4, 2022 14:44
@jmillan jmillan merged commit 0a67781 into v3 Nov 4, 2022
@jmillan jmillan deleted the hex-data-representation-normalization branch November 4, 2022 16:30
@nazar-pc
Copy link
Collaborator

nazar-pc commented Nov 4, 2022

It wasn't necessarily bad before. If something in the spec defines how bits should be arranged, it should be in binary in the codebase as well so it is readable. When you replace it with hex, you can't see with your eyes immediately what individual bits are. I guess whoever was writing it before was going from the spec and that is why it was the way it was.

@jmillan
Copy link
Member Author

jmillan commented Nov 5, 2022

While I understand your rationale, and I share it to some extent, there were up to three different ways to represent the content in the same buffer (hex, binary, decimal), which does not help readability. Less if there is no comment supporting what the value is representing.

I think it's sometimes desirable to see the binary value of a buffer position for the reasons you give, and we can add it if/when needed. On the other hand it's common for editors to show you a representation of a given value in different encodings, so I can see the binary, octal or decimal value of the hex value under the cursor.

piranna pushed a commit to dyte-in/mediasoup that referenced this pull request Feb 9, 2023
* tests: normalize hexadecimal data representation

Each byte is represented as 0x%02x separated with comma and space within
each other.

* Update CHANGELOG
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants