Skip to content

SocketDiagTCPInfo: Add support for BBR information#572

Merged
aboch merged 1 commit intovishvananda:masterfrom
taktv6:master
Nov 26, 2020
Merged

SocketDiagTCPInfo: Add support for BBR information#572
aboch merged 1 commit intovishvananda:masterfrom
taktv6:master

Conversation

@taktv6
Copy link
Copy Markdown

@taktv6 taktv6 commented Oct 1, 2020

No description provided.

@taktv6
Copy link
Copy Markdown
Author

taktv6 commented Oct 12, 2020

Any chance I could get some feedback?

@vishvananda
Copy link
Copy Markdown
Owner

This is looking good to me. Any way to add a test for the functionality? Also, when you are ready to go, please squash your changes down into a single commit.

@taktv6
Copy link
Copy Markdown
Author

taktv6 commented Oct 20, 2020

Thank you for your feedback. I've refactored the SocketDiagTCPInfo function and added some tests.
Unfortunately testify doesn't support Go 1.12 anymore. What would you like me to use as an alternative?

@leoluk
Copy link
Copy Markdown

leoluk commented Oct 30, 2020

This would be super useful to have - any news on this?

@taktv6
Copy link
Copy Markdown
Author

taktv6 commented Nov 9, 2020

@vishvananda feedback pls?

@aboch
Copy link
Copy Markdown
Collaborator

aboch commented Nov 24, 2020

LGTM

Please squash these changes in one commit ("Squash and merge" is not enabled here).

@taktv6
Copy link
Copy Markdown
Author

taktv6 commented Nov 24, 2020

Okay. But any idea about the testify issue?
See #596

@aboch
Copy link
Copy Markdown
Collaborator

aboch commented Nov 24, 2020

But any idea about the testify issue?

Yes, I'd suggest to remove the testify import from these changes. Check the struct equality in the test code.

I would stick to the builtin go testing package, because of the simplicity of the functions that needs to be tested here and for keeping this small library code base lean.

@taktv6
Copy link
Copy Markdown
Author

taktv6 commented Nov 24, 2020

Okay. I'll sort that out tomorrow. Thanks!

@aboch
Copy link
Copy Markdown
Collaborator

aboch commented Nov 26, 2020

LGTM

@aboch aboch merged commit 85be443 into vishvananda:master Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants