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

TCP protocol support. #538

Merged

Conversation

firerain-fd
Copy link
Contributor

TCP protocol support has been added.
Updated tests.

#537

@firerain-fd firerain-fd requested a review from a team as a code owner August 23, 2023 16:00
@martincostello martincostello linked an issue Aug 23, 2023 that may be closed by this pull request
Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. It looks like some of the tests are failing though.

tests/JustEat.StatsD.Tests/Utf8TagsFormatterTests.cs Outdated Show resolved Hide resolved
Unused namespaces have been removed.
@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9d083ff) 83.18% compared to head (b61a358) 83.77%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #538      +/-   ##
==========================================
+ Coverage   83.18%   83.77%   +0.59%     
==========================================
  Files          28       28              
  Lines         666      672       +6     
  Branches      150      151       +1     
==========================================
+ Hits          554      563       +9     
+ Misses         70       67       -3     
  Partials       42       42              
Flag Coverage Δ
linux 83.18% <100.00%> (+0.60%) ⬆️
macos 83.48% <100.00%> (+1.04%) ⬆️
windows 83.18% <100.00%> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martincostello
Copy link
Member

Thanks for your contribution @firerain-fd, this looks good.

Could you confirm that you've tried this out with the use cases you think you'd use it for and that is fulfills those needs? Gives us some extra confirmation beyond whatever is in our test suite.

Other than that, I'm not going to merge this yet as we've been rather lax with actually releasing all the pent-up change in our main branch for a while and that's some admin I need to work through when I get the time to sort it out. Once things are prepped for a new release, then I'll merge this and we can look at finally shipping a new version.

@martincostello martincostello self-assigned this Aug 24, 2023
@martincostello martincostello added this to the v5.0.0 milestone Aug 24, 2023
@firerain-fd
Copy link
Contributor Author

Yes, I checked this in my project. Now I can get an exception when it is impossible to connect to statsd server or when I fail to send a request.

@martincostello martincostello merged commit 73e564e into justeattakeaway:main Dec 4, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding TCP protocol
2 participants