Fix the implementation for the multi payload
tested on true workflow
a part of #385
and will fix #409
@1c3t3a please take a few minute to view this PR
Codecov Report
Attention: Patch coverage is 73.68421% with 5 lines in your changes are missing coverage. Please review.
Project coverage is 91.80%. Comparing base (
decb053) to head (a4f04d8).
:exclamation: Current head a4f04d8 differs from pull request most recent head 31f0fe3. Consider uploading reports for the commit 31f0fe3 to get more accurate results
| Files | Patch % | Lines |
|---|---|---|
| socketio/src/client/raw_client.rs | 75.00% | 3 Missing :warning: |
| socketio/src/asynchronous/client/client.rs | 71.42% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #392 +/- ##
==========================================
- Coverage 91.80% 91.80% -0.01%
==========================================
Files 36 36
Lines 5124 5123 -1
==========================================
- Hits 4704 4703 -1
Misses 420 420
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hey @shenjackyuanjie, I appreciate your interest in this repository and project. I see you identified a few things that we can improve upon and a few features we can implement, which is great!
In order to keep the code maintainable and healthy, I highly value small and easily reviewable chunks of code (reading PRs). Also every new code must come with sufficient testing and should be of reasonable quality.
For the changes you do to the project, I noticed you don't necessarily follow that approach. For example this PR contains a bunch of different feature implementations. With this it is not possible for me to review that code and for you it reduces the chance of ever merging the feature.
To move forward I advise you to create small increments of high quality code (you know what each line of code you submit does and why it needs to be there) for me to review. Also please break things down in smaller features and add sufficient tests for everything, otherwise I will reject patches.
ah, I'm sorry with the last commit, I forget it's still on this PR I will reset the branch back
Could you please add a test or further explain what exactly that code enables? :)
the first is just a leftover comment, there aren't any unwrap
I add more comment for the second one
@1c3t3a please have a check of the code and the comment
Can you please add a test for the behavior you're changing here? :)
code coverage faild …… weird, but the test do passed
well…… I'll clean them up tonight
the code coverage still broke ╮( ̄⊿ ̄)╭
@1c3t3a I think you may need a fix for it
@1c3t3a I think you may need a fix for it
Oke, it doesn't matter for now, we can fix this separately. Thanks for the PR!