Abstract aborter into its own library#2373
Closed
bterlson wants to merge 25 commits into
Closed
Conversation
weshaggard
reviewed
Apr 24, 2019
bterlson
force-pushed
the
rush_abstract_aborter
branch
from
07552de to
dc5f0d8
Compare
Member
Author
|
Now that we've landed the rush change, this is good for a review. If the approach seems good, we can finish this up by:
|
jeremymeng
reviewed
May 1, 2019
| "types": "./types", | ||
| "keywords": [], | ||
| "author": "", | ||
| "license": "ISC", |
Member
There was a problem hiding this comment.
I guess this is from npm init and we use MIT?
Member
Author
There was a problem hiding this comment.
good catch, also need LICENSE file.
jeremymeng
reviewed
May 1, 2019
jeremymeng
reviewed
May 1, 2019
|
|
||
| allTasksSignal.abort(); // aborts allTasksSignal, subTask1, subTask2 | ||
| subTask1.abort(); // aborts only subTask1 | ||
| ``` |
Member
There was a problem hiding this comment.
should also mention the consequences of aborting?
Member
Author
There was a problem hiding this comment.
Agreed, I'll add an example about that.
jeremymeng
reviewed
May 1, 2019
Member
|
browser tests are failing for me. Maybe due to rollup issue. |
…to abstract_aborter_test
removed unused rollup json plugin
* Rename clients from *URL => *Client * Rename test files after the client rename
* [Storage-file] Rename *URL to *Client * Fix sample code in README.md * Rename test files after client rename
* [Storage-queue] Rename *URL to *Client * Rename test files after client rename
This is a follow up on previous rename refactoring.
- rush sync-versions - add "assert" dev dependency - fix rollup this null warning
Contributor
|
Just noticed that this PR is still open, should it be closed now that @chradek finished this work? |
Member
Author
|
Yuppp, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This draft PR builds on #2047 and abstracts Aborter into its own package and updates storage-blob to use this package.