-
Notifications
You must be signed in to change notification settings - Fork 161
Add Zip
for multiple Publishers
#6
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6 +/- ##
==========================================
+ Coverage 94.27% 94.86% +0.58%
==========================================
Files 22 24 +2
Lines 908 1012 +104
==========================================
+ Hits 856 960 +104
Misses 52 52
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jasdev, thank you so much for making this!
I've had a couple of rough days with the entire COVID crisis, sorry this took a bit longer than expected.
This looks really good, I left a couple of minor notes / thoughts that I'd love your thoughts on. LMK!
Co-Authored-By: Shai Mishali <freak4pc@gmail.com>
Co-Authored-By: Shai Mishali <freak4pc@gmail.com>
Co-Authored-By: Shai Mishali <freak4pc@gmail.com>
No worries at all! Hope any dust has settled and that y’all are safe. Never any rush to reply—it’s already inspiring that you’re able to find the time, even as a new parent. I think I covered everything (sorry for the force push, had a rebasing snag)! Let me know if there’s anything else blocking this PR, when you get some headspace. 🙏🏽 |
I'm a fan of force-push on feature branches, no need to apologize :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some final nits but this looks awesome, ready from my side.
I think i'll just commit the nits and merge this in.
ZipMany.swift
, tests, and documentation.Zip
for multiple Publishers
Rough sketch of overloads to allow folks to zip arbitrarily many publishers. I tried to mirror the existing style in the repository, but please feel free to send any and all feedback (no nit too small)! 🙏🏽