Skip to content
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

Added an event that is emitted when a channel is created. #2711

Merged
merged 5 commits into from
Jan 18, 2018

Conversation

Gongreg
Copy link
Member

@Gongreg Gongreg commented Jan 10, 2018

Issue: Need ability to know when a channel was created between server and manager.

What I did

Added an event (copied logic from #1639).

@Gongreg Gongreg changed the title [React Native] Added an event that is emitted when a channel is created. Added an event that is emitted when a channel is created. Jan 10, 2018
@codecov
Copy link

codecov bot commented Jan 10, 2018

Codecov Report

Merging #2711 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2711      +/-   ##
==========================================
- Coverage   35.77%   35.75%   -0.02%     
==========================================
  Files         433      433              
  Lines        9438     9441       +3     
  Branches      981     1005      +24     
==========================================
  Hits         3376     3376              
+ Misses       5417     5376      -41     
- Partials      645      689      +44
Impacted Files Coverage Δ
app/react-native/src/preview/index.js 0% <0%> (ø) ⬆️
app/react/src/client/manager/provider.js 0% <0%> (ø) ⬆️
app/vue/src/client/manager/provider.js 0% <0%> (ø) ⬆️
app/react-native/src/manager/provider.js 0% <0%> (ø) ⬆️
app/angular/src/client/manager/provider.js 0% <0%> (ø) ⬆️
app/polymer/src/client/manager/provider.js 0% <0%> (ø) ⬆️
app/react/src/server/config/babel.js 0% <0%> (-100%) ⬇️
app/react/src/server/babel_config.js 0% <0%> (-76.67%) ⬇️
app/react/src/server/utils.js 0% <0%> (-53.58%) ⬇️
.../src/modules/ui/components/stories_panel/header.js 29.62% <0%> (ø) ⬆️
... and 56 more

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 b58df08...52ea398. Read the comment docs.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks pretty reasonable to me. @tmeasday any thoughts?

@@ -42,16 +47,6 @@ export default class ReactProvider extends Provider {
const renderPreview = addons.getPreview();

const innerPreview = renderPreview ? renderPreview(kind, story) : null;

if (this.options.manualId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gongreg Was this just for debugging? Any implications to removing it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shilman Kinda. That is what I used before to connect phone with browser.

@tmeasday
Copy link
Member

Seems reasonable to me. Can we use this to fix #1192?

@shilman
Copy link
Member

shilman commented Jan 13, 2018

@tmeasday @Gongreg interesting idea about #1192. Do we want to make this event in the other frameworks as well (React, Vue, Angular) as part of this PR?

@Gongreg
Copy link
Member Author

Gongreg commented Jan 15, 2018

@shilman we should try it out and create a separate pr i guess. :)
Maybe we should create an issue so we wouldn't forget?

@shilman shilman merged commit 694d5d9 into master Jan 18, 2018
@shilman shilman deleted the channel-created-event branch January 18, 2018 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants