Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

sock_diag: wire up #99

Merged
merged 7 commits into from
Sep 26, 2020

Conversation

george-hopkins
Copy link
Contributor

As I needed to quickly diagnose some sockets, I took the liberty to finish up your work in #56. Specifically, I added a buffer for the messages and an example. Other than a small fix of the serialization of port numbers, it's just a rebased version of your implementation. I'm happy to close this PR and rebase my fixes on your work in case you'd like to keep working on it.

@little-dude
Copy link
Owner

Hi @george-hopkins this is very cool thank you! I'll take a look.

I'm happy to close this PR and rebase my fixes on your work in case you'd like to keep working on it.

Well at this point there have already been two attempts to land this. The problem is that it's massive. So maybe we should change strategy and merge a first implementation even if it's incomplete, and work from there. So I'm tempted to try to merge this already. Wdyt?

@george-hopkins
Copy link
Contributor Author

Thank you, it's quite a big addition indeed. However, I think it can serve as a good base due to the good architecture which handles unparsed attributes quite nicely. In addition, I think it will lower the bar for improvements and encourage other developers to contribute the remaining bits over time if the base is already laid out and can serve as an example.

@little-dude
Copy link
Owner

All right. Let merge this in then :)

@little-dude little-dude merged commit 2f312d8 into little-dude:master Sep 26, 2020
@@ -0,0 +1,25 @@
[package]
authors = ["Flier Lu <flier.lu@gmail.com>", "Corentin Henry <corentinhenry@gmail.com>"]
Copy link
Owner

Choose a reason for hiding this comment

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

@george-hopkins I'd like to release 0.1, and noticed that you haven't added yourself to the authors. Would you like to? If so which name/email would you like to use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for asking! It's fine by me, I'm glad sock_diag is officially supported now 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants