Skip to content
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

Turn vsync into a core service. #1238

Merged
merged 1 commit into from
Dec 23, 2015
Merged

Conversation

cramforce
Copy link
Member

Avoids loading it redundantly into extensions.

Part of #1055

@@ -28,4 +29,5 @@ export function installCoreServices(window) {
installViewerService(window);
installViewportService(window);
installHistoryService(window);
installVsyncService(window);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like vsync should be one of the first to be installed, or maybe right after viewer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have these install each other when needed, so that order here doesn't matter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. LGTM.

@dvoytenko dvoytenko self-assigned this Dec 23, 2015
@dvoytenko
Copy link
Contributor

Thanks! A couple of questions, but otherwise LGTM.

@cramforce
Copy link
Member Author

The increase in the core binary is because of one extra module. This could
be compiled away with a better compiler like closure or a custom pass to
browserify.
On Dec 23, 2015 9:50 AM, "Dima Voytenko" notifications@github.com wrote:

Thanks! A couple of questions, but otherwise LGTM.


Reply to this email directly or view it on GitHub
#1238 (comment).

Avoids loading it redundantly into extensions.

Part of ampproject#1055
cramforce added a commit that referenced this pull request Dec 23, 2015
Turn vsync into a core service.
@cramforce cramforce merged commit c11ed15 into ampproject:master Dec 23, 2015
@cramforce cramforce deleted the vsync-service branch December 23, 2015 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants