diegohaz / redux-saga-thunk

Dispatching an action handled by redux-saga returns promise
MIT License
221 stars 17 forks source link

#13 - Added isDone and isComplete selector feature #16

Closed sebastianjg closed 7 years ago

sebastianjg commented 7 years ago

I encountered a case where it was suitable to solve with a hasSuccess-selector, as mentioned in #13, so I went ahead and added it.

Excellent work with the Arc repository, really appreciate it!

codecov-io commented 7 years ago

Codecov Report

Merging #16 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #16   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines          70     80   +10     
  Branches       23     25    +2     
=====================================
+ Hits           70     80   +10
Impacted Files Coverage Δ
src/reducer.js 100% <100%> (ø) :arrow_up:
src/selectors.js 100% <100%> (ø) :arrow_up:

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 f666089...1dfaf88. Read the comment docs.

diegohaz commented 7 years ago

Hi, @sebastianjg. Excellent work. Thank you so much.

I'm just not sure about the name. My English isn't the best, but hasSuccess seems strange. Do people say it has success...?

What do you think?

cc/ @Geczy

Geczy commented 7 years ago

http://api.jquery.com/jquery.ajax/

diegohaz commented 7 years ago

isDone seems good.

Geczy commented 7 years ago

What about just pending, failed, done, complete?

Without the prefix?

diegohaz commented 7 years ago

@Geczy I like it, but let's discuss that on another thread. For this PR, just changing hasSuccess to isDone is 👍.

@sebastianjg

sebastianjg commented 7 years ago

Thanks for the feedback guys! I agree that isDone is better (a bit hasty on my end as a non-native english speaker).

I have updated the PR.

edit: I also added a isComplete selector, which will be true when it is either a failure or success.

diegohaz commented 7 years ago

Thank you, @sebastianjg