Migrate to DI #32
Migrate to DI #32
Conversation
|
Found TODOs without GitHub issues: |
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
=========================================
+ Coverage 86.4% 87.5% +1.09%
=========================================
Files 3 4 +1
Lines 103 112 +9
Branches 19 16 -3
=========================================
+ Hits 89 98 +9
- Misses 6 7 +1
+ Partials 8 7 -1
Continue to review full report at Codecov.
|
README.md
Outdated
| @@ -1,4 +1,4 @@ | |||
| # fusion-plugin-universal-events | |||
| # fusion-plugin-universal-events | |||
| const scoped = events.from(ctx); | ||
| ``` | ||
|
|
||
| Returns a scoped version of the events api. |
There was a problem hiding this comment.
Explain why one would want to use this over the global instance. Document API differences between a global emitter and a scoped one.
There was a problem hiding this comment.
That should be explained in the "Accessing ctx section
README.md
Outdated
|
|
||
| Returns a scoped version of the events api. | ||
|
|
||
| - `ctx: FusionContext` - Required. |
There was a problem hiding this comment.
Should link to docs about FusionContext
There was a problem hiding this comment.
How are we planning on doing those links when we have to handle both open source and closed source documentation?
| return new ScopedEmitter(ctx, this); | ||
| }); | ||
| } | ||
| emit(type, payload, ctx) { |
There was a problem hiding this comment.
Under what use cases would one use the 3rd arg here, and under what use cases would one use emitter.from? Is there a strong enough motivation to have both be part of the API?
There was a problem hiding this comment.
This is purely implementation detail, and not exposed
README.md
Outdated
| @@ -1,4 +1,4 @@ | |||
| # fusion-plugin-universal-events | |||
| # fusion-plugin-universal-events | |||
|
|
|||
| [](https://buildkite.com/uberopensource/fusion-plugin-universal-events) | |||
|
|
|||
There was a problem hiding this comment.
There should be a section talking about why we didn't use an existing event emitter library and how the API and semantics of our library differs from other event emitters.
If this library is not a suitable implementation for a hypothetical EventEmitterToken interface, we should also mention that.
|
!merge |
Fixes #30