Skip to content

Schedule with owner blocked on build#9169

Merged
dvoytenko merged 2 commits intoampproject:masterfrom
dvoytenko:fixes166
May 5, 2017
Merged

Schedule with owner blocked on build#9169
dvoytenko merged 2 commits intoampproject:masterfrom
dvoytenko:fixes166

Conversation

@dvoytenko
Copy link
Copy Markdown
Contributor

Closes #8713.

@dvoytenko dvoytenko requested a review from zhouyx May 5, 2017 17:56
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

Was the previous code expecting owner to call #build?

scheduleLayoutOrPreloadForSubresources_(parentResource, layout, subElements) {
const resources = [];
this.discoverResourcesForArray_(parentResource, subElements, resource => {
if (resource.getState() != ResourceState.NOT_BUILT) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please flip the conditional.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@dvoytenko
Copy link
Copy Markdown
Contributor Author

@jridgewell build is always called automatically, as soon as upgrade happens.

@dvoytenko dvoytenko merged commit e26d22a into ampproject:master May 5, 2017
@dvoytenko dvoytenko deleted the fixes166 branch May 5, 2017 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants