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

gulp performance: emit status updates #27030

Merged
merged 1 commit into from
Mar 3, 2020

Conversation

samouri
Copy link
Member

@samouri samouri commented Feb 28, 2020

summary
I recently started taking advantage of the new gulp performance task. During its execution, it always emits a message about calling yarn install, but then appears to hang until all the measurements are complete. For a single link with 100 runs, this meant 12m without a status update.

This PR adds progress notification to the task. Here is an example of the output:
Screen Shot 2020-02-28 at 6 19 16 PM

@samouri samouri requested a review from kevinkimball February 28, 2020 23:19
@samouri samouri self-assigned this Feb 28, 2020
@amp-owners-bot amp-owners-bot bot requested a review from rcebulko February 28, 2020 23:19
@rcebulko rcebulko requested a review from estherkim March 2, 2020 16:35
Copy link
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Esther is working on integrating this task into the Travis CI, so while this looks like a step forward I'm going to hold off on approving as she may have plans to implement logging a different way/using amphtml logging conventions.

Since you're using the gulp performance task, it would be super helpful to file any bugs or issues you encounter as issues and tag @ampproject/wg-infra or Esther directly, to help with planning work around it and to get a picture of what could/should be done.

Copy link
Collaborator

@estherkim estherkim left a comment

Choose a reason for hiding this comment

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

Thanks @samouri!

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.

5 participants