-
Notifications
You must be signed in to change notification settings - Fork 85
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
MVP: Imperative Event Emitter #2118
Conversation
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: Amber Torrise <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
…-mvp Signed-off-by: zFernand0 <[email protected]>
packages/imperative/__tests__/__integration__/ImperativeEventEmitter.integration.test.ts
Fixed
Show fixed
Hide fixed
…integration__ Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2118 +/- ##
==========================================
+ Coverage 91.15% 91.20% +0.05%
==========================================
Files 623 628 +5
Lines 17716 17877 +161
Branches 3763 3750 -13
==========================================
+ Hits 16149 16305 +156
- Misses 1566 1571 +5
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
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.
Looks great so far! Going to run some tests locally, but the logic makes sense to me. I left some feedback regarding some of the types and the CodeQL warning.
packages/imperative/src/events/src/doc/IImperativeEventParms.ts
Outdated
Show resolved
Hide resolved
packages/imperative/src/events/src/doc/IImperativeEventParms.ts
Outdated
Show resolved
Hide resolved
Thanks for the review! 🙏🏽 |
…-mvp Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
...s/imperative/src/events/__tests__/__integration__/ImperativeEventEmitter.integration.test.ts
Fixed
Show fixed
Hide fixed
Signed-off-by: zFernand0 <[email protected]>
packages/imperative/src/events/__tests__/__unit__/ImperativeEventEmitter.unit.test.ts
Fixed
Show fixed
Hide fixed
Signed-off-by: zFernand0 <[email protected]>
Signed-off-by: zFernand0 <[email protected]>
This PR should be ready to review as-is. There will be many more changes happening once the following PR gets in: |
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 made all of my comments while this PR was still draft, and most of those comments were just questions.
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.
LGTM, thanks for all your work on this and for addressing feedback 😁 This is a nice stepping stone for Zowe Client apps and extensions to keep in sync with one another 😋
Signed-off-by: zFernand0 <[email protected]>
Quality Gate passedIssues Measures |
Merging in order to get the MVP out in its own prerelease version ( |
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
How to Test
You may need to create a dummy app (e.g. VSCE) in order to test this 😋
( or use
- amber's comment )fernando's example VSCE
UPDATE:
Here is a public instance of the sample VSCE
Review Checklist
I certify that I have:
Additional Comments
TODO:
packages/imperative/src/events/src/ImperativeEventConstants.ts
)Expand this section for a complete list of future enhancements