Closed dmicic closed 4 years ago
Merging #40 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #40 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 6 6
Lines 78 87 +9
Branches 11 11
=====================================
+ Hits 78 87 +9
Impacted Files | Coverage Δ | |
---|---|---|
src/EventBus.ts | 100% <100%> (ø) |
:arrow_up: |
src/useBusState.ts | 100% <100%> (ø) |
:arrow_up: |
src/react.ts | 100% <100%> (ø) |
:arrow_up: |
src/useBusReducer.ts | 100% <100%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 21cf432...23ac33d. Read the comment docs.
@ryardley not sure whether the names of the new functions are more meaningful than the one proposed in the issue #37
Ping
Sorry been trekking through the Sahara!
@ryardley not sure whether the names of the new functions are more meaningful than the one proposed in the issue #37
Just a thought maybe we can sit on it for a bit and don't need them in this PR?
Solution for #37 but with more flexible subscription option. Not fully finalized yet.
@ryardley have a look at test should reduce using multiple event subscription types
Hmm initial observations are that I think it’s a little confusing to my head and I would probably do it differently but if you think that is the simplest way to do it and you think it’s tested enough then I guess it is ok for now. Not sure staying in arrayland makes heaps of sense unless you can see big performance increases. A function that takes a whatever and a handler and does a subscription can always be mapped over an array. It’s quick to test for an array and create one with the input too. Anyway it will be a few days before I am on my computer coding but will try and deliver some more feedback then.
On Sun, 12 Jan 2020 at 01:43, Darko Micic notifications@github.com wrote:
@ryardley https://github.com/ryardley have a look at test should reduce using multiple event subscription types
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/ryardley/ts-bus/pull/40?email_source=notifications&email_token=AAJSXWOUKMMHCUS7RP4S4BDQ5JRR3A5CNFSM4KCTOOEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIWOQRI#issuecomment-573368389, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJSXWK3JS6UWIFR2I34N6DQ5JRR3ANCNFSM4KCTOOEA .
--
RUDI YARDLEY Online Services & Software Engineering
Email: contact@rudiyardley.com Website: http://www.rudiyardley.com Github: http://github.com/ryardley LinkedIn: http://linkedin.com/in/rudiyardley Twitter: http://twitter.com/rudiyardley Skype: @rudi.yardley Mobile: +61401427141
The solutions all are simple - after you have arrived at them. But they're simple only when you know already what they are. - Robert M. Pirsig
@ryardley no worries. We don’t need to rush. I am not emotionally bound to that approach :D feedback is welcome. Let’s have a look once u have more time. I chose this approach as it had a limited impact in the code. Curious to hear what you have in mind
Implementation of #37