-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
This is the very first attempt at making Let me know what you think. I'll probably do the same thing with |
Thank you @mderriey. How much does this speed things up? |
I only tested it on Octokit, I don't know if it qualifies as a good subject for testing. It did speed things up, however that's based on my personal judgment only, so I don't have any metrics to share. Do you have a codebase you know know takes some time? I could give it a try, and maybe try to time things. |
@ctaggart could you let me know what you'd want done to move this forward? |
@mderriey Some kind of test that shows that performance is improved. Using Octokit with Travis CI or AppVeyor would work. You can simply create a branch that uses a build from this PR. https://github.com/ctaggart/SourceLink/blob/v2/NuGet.Config
|
I can't seem to find a build for this PR on AppVeyor. Am I missing something? |
@mderriey Weird. I just updated the webhook to send pull requests too. Can you push again to your branch? |
OK so I compared AppVeyor builds times. Before changes:
After changes - using the
The links go to the relevant lines of the builds that show the time taken for the Cake task that runs |
That is great! Did you have more changes to make? |
Unless you can think of something else we could try to improve, no. I didn't put much thought into the design of the changes, either, just went async where you said we could do it. Feel free to review the changes in depth and let me know if you see anything you might want done another way. |
Testing a Rx.NET package on my laptop drops the test time from 50 seconds to 13 seconds. Once again, great job. dotnet/reactive#393 |
This will be in version 2.1.1. |
Fixes #190