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

Update brcast to ^3.0.1 #38

Closed
wants to merge 1 commit into from
Closed

Conversation

darkowic
Copy link

@darkowic darkowic commented Sep 14, 2017

BREAKING CHANGE: themeListener.subscribe returns subscriptionId intead of function. To unsubscribe you have to call themeListener.unsubscribe(subscriptionId)

Resolves #36

BREAKING CHANGE: themeListener.subscribe returns subscriptionId intead of function. To unsubscribe you have to call themeListener.unsubscribe(subscriptionId)
Fix tests
@darkowic
Copy link
Author

@oliviertassinari Can you check this?

@oliviertassinari
Copy link
Contributor

oliviertassinari commented Sep 14, 2017

@darkowic I have been reducing the dependency on react-jss. By directly importing required modules, I would expect theming to no longer be required.
(mui/material-ui#8121)

@coveralls
Copy link

coveralls commented Sep 14, 2017

Coverage Status

Coverage decreased (-0.7%) to 92.92% when pulling 2925fb3 on darkowic:master into ded4545 on iamstarkov:master.

themeListener.unsubscribe(this.context, this.subscriptionId);
}
```
```
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't be there, righT?

@@ -91,7 +91,7 @@
"rimraf": "^2.6.1"
},
"dependencies": {
"brcast": "^2.0.0",
"brcast": "^3.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

this is an actual change corresponding the pr, or is there something else?

@@ -18,13 +18,20 @@ export default function createThemeListener(CHANNEL = channel) {

function subscribe(context, cb) {
if (context[CHANNEL]) {
return context[CHANNEL].subscribe(cb);
return context[CHANNEL].subscribe(cb);
Copy link
Member

Choose a reason for hiding this comment

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

formatting fixes should always go separate so that we know what has happened in a commit

@@ -7,3 +7,4 @@ dist
shrinkwrap.*
.nyc_output
coverage
.idea
Copy link
Member

Choose a reason for hiding this comment

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

completely unrelated

@kof
Copy link
Member

kof commented Oct 14, 2017

@darkowic I know its a long due, can you please clean up the pr, separate the commits or even better send separate PRs?

@hburrows hburrows closed this Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants