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

Changed reporters to be lazily loaded #27

Merged
merged 1 commit into from Apr 18, 2015

Conversation

Projects
None yet
2 participants
@robdmoore
Contributor

robdmoore commented Apr 18, 2015

Fixes #20 and results in a massive performance improvement in Windows

Changed reporters to be lazily loaded
Fixes #20 and results in a massive performance improvement in Windows
@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Apr 18, 2015

Contributor

I tried this out on my project and it worked great! Huge performance improvement.

If you have a bunch of failures then they will still be slow, but I figure that's the rarer case and could still be addressed in the future. This fixes my immediate problems.

Contributor

robdmoore commented Apr 18, 2015

I tried this out on my project and it worked great! Huge performance improvement.

If you have a bunch of failures then they will still be slow, but I figure that's the rarer case and could still be addressed in the future. This fixes my immediate problems.

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Apr 18, 2015

Contributor

Also, FYI the two approval files in the test project fail on Windows due to line endings differences. Might be worth storing them as binary via gitattributes.

Contributor

robdmoore commented Apr 18, 2015

Also, FYI the two approval files in the test project fail on Windows due to line endings differences. Might be worth storing them as binary via gitattributes.

staxmanade added a commit that referenced this pull request Apr 18, 2015

Merge pull request #27 from robdmoore/fix-windows-perf
Changed reporters to be lazily loaded

@staxmanade staxmanade merged commit ac450d0 into approvals:master Apr 18, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@staxmanade

This comment has been minimized.

Show comment
Hide comment
@staxmanade

staxmanade Apr 18, 2015

Member

Thanks! Clean!

Member

staxmanade commented Apr 18, 2015

Thanks! Clean!

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Apr 18, 2015

Contributor

Thanks for the merge!

When do you think this might hit npm?

Contributor

robdmoore commented Apr 18, 2015

Thanks for the merge!

When do you think this might hit npm?

@staxmanade

This comment has been minimized.

Show comment
Hide comment
@staxmanade

staxmanade Apr 19, 2015

Member

Let me know how this version looks.... 0.0.27

Member

staxmanade commented Apr 19, 2015

Let me know how this version looks.... 0.0.27

@robdmoore robdmoore deleted the robdmoore:fix-windows-perf branch Apr 19, 2015

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Apr 19, 2015

Contributor

Works great! thanks so much! :)

Contributor

robdmoore commented Apr 19, 2015

Works great! thanks so much! :)

@robdmoore robdmoore referenced this pull request Apr 19, 2015

Closed

Update to approvals@0.0.27 #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment