Skip to content

/ vuex

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

Jump to bottom

Allow action subscribers to catch rejections. #1740

Merged
merged 9 commits into from May 8, 2020
Merged

Allow action subscribers to catch rejections. #1740

merged 9 commits into from May 8, 2020

Conversation

@jmplahitko
Copy link
Contributor

@jmplahitko jmplahitko commented Apr 30, 2020

@kiaking - Thanks for the direction. This is the pull request to replace #1531
Joe Plahitko added 6 commits Apr 8, 2019
Joe Plahitko
Allow action subscribers to catch rejections.
Loading status checks…
42752e1
Joe Plahitko
Removed dist folder and wrapped dispatch return in promise to allow f…
Loading status checks…
ab5e213 
…or rejections without a throw.
Joe Plahitko
Fixed missing dist folder that breaks test suite.
Loading status checks…
9b2dcce
Joe Plahitko
Merge branch 'dev' of https://github.com/vuejs/vuex into vuejs-dev
4b8ba78
Joe Plahitko
Updated ActionSubscriberObject.catch to ActionSubscriberObject.error
4f13ee4
Joe Plahitko
Replaced explicit process.env checks with environment variable
Loading status checks…
dccfee5
@kiaking
kiaking requested changes May 1, 2020
View changes
Copy link
Member

@kiaking kiaking left a comment

Thanks a lot! It's looking good to me. But, there's still dist folder in the commit 😓 Could you please remove it 🙏
@kiaking kiaking added the proposal label May 1, 2020
Joe Plahitko
Removing dist folder.
Loading status checks…
90bc4d7
@cuebit
Copy link
Member

@cuebit cuebit commented May 1, 2020 •
edited

Sorry @jmplahitko, I don't think we've been very clear in what we're asking of you, and I apologise for that.

When members ask to have dist removed from a commit, generally it means avoiding adding any files modified in the dist directory to the commit.

If it makes things easier, 1) take the dist directory from dev and throw it back in, 2) commit dist, and 3) run tests again but this time don't include any modifications in dist.

Hope that makes a little more sense.
Joe Plahitko
Recommit dev dist.
Loading status checks…
151e4e2
@jmplahitko
Copy link
Contributor Author

@jmplahitko jmplahitko commented May 5, 2020

@kiaking - Not your fault, I misunderstood. Hopefully that last commit gets us there. Thanks.
@cuebit cuebit requested a review from kiaking May 5, 2020
@marswong
marswong reviewed May 6, 2020
View changes
src/store.js Outdated Show resolved Hide resolved
Joe Plahitko
Updated error message in error action subscriber.
Loading status checks…
d680763
@kiaking
kiaking approved these changes May 8, 2020
View changes
Copy link
Member

@kiaking kiaking left a comment

Looking good! Thanks so much for the work. We know we need to fix the test because we've just switched to using Jest, but I'll fix that right after merging this one 👍
@kiaking kiaking merged commit 6ebbe64 into vuejs:dev May 8, 2020
6 checks passed
6 checks passed
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: lint-types Your tests passed on CircleCI!
Details
ci/circleci: test-e2e Your tests passed on CircleCI!
Details
ci/circleci: test-ssr Your tests passed on CircleCI!
Details
ci/circleci: test-unit Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview ready!
Details
This was referenced May 8, 2020
Closed
Closed
Merged
@sodatea sodatea mentioned this pull request Sep 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marswong marswong

@kiaking kiaking

Assignees
No one assigned
Labels
proposal
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@jmplahitko @cuebit @kiaking @marswong
You can’t perform that action at this time.