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.

Sign up for GitHub

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

Jump to bottom

BREAKING: Remove symbol from type #86

Merged
merged 6 commits into from
Nov 2, 2017
Merged

BREAKING: Remove symbol from type #86

merged 6 commits into from
Nov 2, 2017

Conversation

unional
Copy link
Contributor

@unional unional commented Oct 27, 2017

Fix #39

@coveralls
Copy link

coveralls commented Oct 27, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 6a56bfa on unional:patch-3 into f509286 on acdlite:master.

@JaKXz
Copy link
Contributor

JaKXz commented Oct 29, 2017

This is something that should be reflected in the exported methods as well.

@JaKXz
Copy link
Contributor

JaKXz commented Oct 30, 2017

I believe this would also resolve #9, if there's general support for it.

JaKXz
JaKXz suggested changes Oct 30, 2017
View reviewed changes
Copy link
Contributor

@JaKXz JaKXz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for taking the time on this, @unional.

I'm just going to mass ping folks, before just going ahead and merging.

@gaearon @acdlite @yangmillstheory @timche @ anyone else!

src/index.js
@@ -7,7 +7,7 @@ import {
export function isFSA(action) {
return (
isPlainObject(action) &&
(isString(action.type) || isSymbol(action.type)) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an import that needs to be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I was editing on github directly. Missed the import. 😛

@coveralls
Copy link

coveralls commented Oct 30, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling 9a5ffeb on unional:patch-3 into f509286 on acdlite:master.

@timche
Copy link
Member

timche commented Nov 2, 2017

Looks good 👍

@JaKXz JaKXz merged commit 6919306 into redux-utilities:master Nov 2, 2017
@unional unional deleted the patch-3 branch November 19, 2017 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JaKXz JaKXz JaKXz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@unional @coveralls @JaKXz @timche