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

(perf) speeding it up. #311

Merged
merged 1 commit into from Jul 5, 2013

Conversation

Projects
None yet
4 participants
@stephenplusplus
Contributor

stephenplusplus commented Jul 3, 2013

(#202)

Hmm, see what you think.

As discussed, require.resolve() is used to find the path of the module, but not load it until necessary.

I don't find the implementation all that pretty, so if you have any suggestions, please share!

@addyosmani

This comment has been minimized.

Show comment
Hide comment
@addyosmani

addyosmani Jul 4, 2013

Member

Will give this some testing today. Thanks for putting together the initial cut of this implementation, Stephen.

Member

addyosmani commented Jul 4, 2013

Will give this some testing today. Thanks for putting together the initial cut of this implementation, Stephen.

@passy

This comment has been minimized.

Show comment
Hide comment
@passy

passy Jul 4, 2013

Member

I didn't review the whole changeset yet, but it is already wayyyy faster for me. Thanks for keeping the ball rolling, Stephen! 👊

Member

passy commented Jul 4, 2013

I didn't review the whole changeset yet, but it is already wayyyy faster for me. Thanks for keeping the ball rolling, Stephen! 👊

@stephenplusplus

This comment has been minimized.

Show comment
Hide comment
@stephenplusplus

stephenplusplus Jul 4, 2013

Contributor

Woo! Good to hear!

Contributor

stephenplusplus commented Jul 4, 2013

Woo! Good to hear!

@addyosmani

This comment has been minimized.

Show comment
Hide comment
@addyosmani

addyosmani Jul 5, 2013

Member

I can confirm the performance improvements are pretty much instant with this change. I'm pleased with the state of the implementation and think we should merge this. It can be retroactively tweaked later on if needed.

Great work, Stephen!

Member

addyosmani commented Jul 5, 2013

I can confirm the performance improvements are pretty much instant with this change. I'm pleased with the state of the implementation and think we should merge this. It can be retroactively tweaked later on if needed.

Great work, Stephen!

addyosmani added a commit that referenced this pull request Jul 5, 2013

Merge pull request #311 from stephenplusplus/speed-it-up
Improve performance by using require.resolve() to find the path of the module, but not load until necessary.

@addyosmani addyosmani merged commit 7630154 into yeoman:master Jul 5, 2013

1 check passed

default The Travis CI build passed
Details

@addyosmani addyosmani referenced this pull request Jul 5, 2013

Closed

Lazy-load generators #202

@passy

This comment has been minimized.

Show comment
Hide comment
@passy

passy Jul 5, 2013

Member

Thanks, Stephen!

Highest of high fives!

Member

passy commented Jul 5, 2013

Thanks, Stephen!

Highest of high fives!

@sindresorhus

This comment has been minimized.

Show comment
Hide comment
@sindresorhus

sindresorhus Jul 13, 2013

Member

This is so gooooood!

Member

sindresorhus commented Jul 13, 2013

This is so gooooood!

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