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

StreamReport :: Limit fetch logs (YN0013) up to 5 #914

Merged
merged 5 commits into from
Feb 7, 2020

Conversation

omergulen
Copy link
Contributor

What's the problem this PR addresses?

This PR is related to #816.

How did you fix it?

I've implemented the request of @arcanis. He advised me to keep the latest 5 fetch (YN0013) logs and use escaping characters while doing it.

I basically saved YN0013 logs by keeping the latest 5 logs in the forgettableLines array.
Then, if it has more than 5 logs, the most aged one is dequeued and I started clearing the console up to 5 lines. And printing them back from the array with remaining lines.

If sequence is interrupted with another type of log, we reset the forgettableLines array.

Also, I kept them internally (private) and put the number 5 in a class-wide private variable, so it can be changed easily.

@omergulen omergulen changed the title StreamReport :: Limit fetch logs up to 5 StreamReport :: Limit fetch logs (YN0013) up to 5 Feb 6, 2020
@arcanis
Copy link
Member

arcanis commented Feb 7, 2020

I've made small tweaks to use constants, but overall this is great - thanks @omergulen!

If you're looking for another task, maybe the "asynchronous tgz conversion" I mentioned on Discord would be a good fit? Basically we have a function that does it, but since it runs with WebAssembly we cannot easily make it asynchronous. The idea would be to instead run the copy operation inside a dedicated short-lived Worker so that the UI thread doesn't block.

@arcanis arcanis merged commit b212835 into yarnpkg:master Feb 7, 2020
@omergulen
Copy link
Contributor Author

Thanks @arcanis! I've seen your changes as well.
Great job!

I will check the new task out soon.

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