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

rust: implement Display and Debug for BStr #893

Closed
wants to merge 1 commit into from
Closed

rust: implement Display and Debug for BStr #893

wants to merge 1 commit into from

Conversation

ohno418
Copy link

@ohno418 ohno418 commented Sep 28, 2022

Implement Display and Debug for BStr. Also, for this purpose, change BStr from a type alias to a newtype wrapper.

Closes #534

Similar PR here, but it seems to be out of date. And a little different from my PR.

Copy link

@adamrk adamrk left a comment

Choose a reason for hiding this comment

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

Seems good to me.

Implement `Display` and `Debug` for `BStr`. Also, for this purpose,
change `BStr`, which previously was a type alias to `[u8]`, to a newtype
of `[u8]`.

Signed-off-by: Yutaro Ohno <[email protected]>
@ohno418 ohno418 changed the title Implement Display and Debug for BStr rust: implement Display and Debug for BStr Oct 1, 2022
@ohno418
Copy link
Author

ohno418 commented Oct 1, 2022

Just improved the commit message, especially adding a subsystem prefix. So no code change with force push.

@KimWang906
Copy link

If this problem is not solved yet, I would like to contribute.

@ojeda
Copy link
Member

ojeda commented Jan 13, 2024

@ohno418 Given we are in mainline, would you like to submit this to the list?

@ohno418
Copy link
Author

ohno418 commented Jan 15, 2024

Ah, yeah. I will submit this patch to the kernel mailing list later.
Thanks for caring about this, @ojeda !

@ojeda
Copy link
Member

ojeda commented Jan 15, 2024

Thanks to you!

Cc'ing @Armavica in case they are still interested in sending their approach too -- it had tests, which was very nice.

Also please consider if a Co-developed-by tag or similar would be appropriate here (https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by) -- perhaps you want to submit this together?

@ohno418
Copy link
Author

ohno418 commented Jan 16, 2024

Also please consider if a Co-developed-by tag or similar would be appropriate here

Sure.

Hi, @Armavica. Would it be okay if I mention you with a Co-developed-by tag in the mailing list? I plan to include the tests you wrote, if that's alright with you.

In case you find that a hassle, I'll just submit my patch to the list as it is.

@Armavica
Copy link

Hi @ohno418, thank you for your message, please feel free to use my tests and I would be happy to be mentioned as Co-developed-by!
Please let me know if there is anything else that I can do.

Copy link
Author

@ohno418 ohno418 left a comment

Choose a reason for hiding this comment

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

@ohno418 ohno418 closed this Jan 25, 2024
@ohno418 ohno418 deleted the impl-debug-display-for-bstr branch January 25, 2024 12:54
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.

Implement Display for CStr and BStr
5 participants