-
Notifications
You must be signed in to change notification settings - Fork 37
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
refactor(viewer.js): Add errorinterceptor argument to propagate error to the top level app #85
refactor(viewer.js): Add errorinterceptor argument to propagate error to the top level app #85
Conversation
@hackermd I do not seem to have permissions to add reviewers to this PR. Please know it's ready for your review. Thanks! |
@hackermd I have addressed the requested changes. Please let me know if this can be merged? Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @GitanjaliChhetri. The changes look good. However, the yarn test
command currently fails for me. The error report includes a lot of linting errors that were discovered by yarn lint
.
Unfortunately, the "unit tests" workflow doesn't automatically get tricked for the pull request. I somehow can't add it as a required status check. cc @fedorov
Until this issue is resolved, the checks need to be run manually locally.
@GitanjaliChhetri @fedorov got the status checks working. |
5bf7586
to
3ecbe48
Compare
@hackermd can you please re-review? Per @GitanjaliChhetri, all of your concerns should now be addressed. |
…ckages (ImagingDataCommons#113)" This reverts commit 2834944.
…py-viewer into addErrorInterceptor
…other packages (ImagingDataCommons#113)"" This reverts commit f5abca9.
Errors thrown in viewer.js can now be propagated to the Slim Viewer App.
Changes needed for ImagingDataCommons/slim#134