rust-socketio icon indicating copy to clipboard operation
rust-socketio copied to clipboard

Implement shared custom data

Open kitgxrl opened this issue 1 year ago • 7 comments

Implements #427

kitgxrl avatar Apr 29 '24 16:04 kitgxrl

Will write tests (if desired) and documentation either when I get home or later tonight!

kitgxrl avatar Apr 29 '24 16:04 kitgxrl

Opening this up for review. I can add tests if they're desired.

kitgxrl avatar Apr 29 '24 18:04 kitgxrl

Codecov Report

Attention: Patch coverage is 21.42857% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 91.49%. Comparing base (fe3c3d8) to head (f27bdbf). Report is 5 commits behind head on main.

:exclamation: Current head f27bdbf differs from pull request most recent head 0175908. Consider uploading reports for the commit 0175908 to get more accurate results

Files Patch % Lines
socketio/src/asynchronous/client/client.rs 12.50% 7 Missing :warning:
socketio/src/client/raw_client.rs 22.22% 7 Missing :warning:
socketio/src/asynchronous/client/builder.rs 20.00% 4 Missing :warning:
socketio/src/client/builder.rs 33.33% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #428      +/-   ##
==========================================
- Coverage   91.86%   91.49%   -0.38%     
==========================================
  Files          36       36              
  Lines        5166     5194      +28     
==========================================
+ Hits         4746     4752       +6     
- Misses        420      442      +22     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 30 '24 07:04 codecov[bot]

Hey @1c3t3a thanks for the review! I'll take a look at the comments made sometime Thursday or Friday due to exams over the next two days (I apologize for the late response).

Both custom_data and shared_data sound good to me, I didn't think about how data could be misleading, thanks for pointing that out. If I think of something better I'll be sure to bring it up!

kitgxrl avatar May 06 '24 23:05 kitgxrl

No worries, there is absolutely now need to apologize! I am super happy you work on this in your free time :)

Good luck with the exams! And in that case I'd personally prefer custom_data, but feel free to come up with something different.

1c3t3a avatar May 07 '24 15:05 1c3t3a

@1c3t3a Took a look at most of what you said, let me know if I missed anything big you'd like to go over. Only thing I believe I haven't looked into yet is using generics on Client and ClientBuilder and accepting a fat pointer on set_data, I'll try and look over this in the next day or two when I have a chance. Apologies for going over this so late haha.

kitgxrl avatar May 15 '24 08:05 kitgxrl

Any chance?

Morb0 avatar Nov 22 '24 21:11 Morb0