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

Remove inaccurate and misleading "memory usage" in dev CLI output #5509

Closed
wants to merge 1 commit into from
Closed

Remove inaccurate and misleading "memory usage" in dev CLI output #5509

wants to merge 1 commit into from

Conversation

koresar
Copy link

@koresar koresar commented Apr 11, 2019

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

After introducing the new DX feature - the "building project" page - the memory usage is printed before the app start. Whereas previously the memory usage was printed after the app start.

image

On the picture above the 58MB is quite inaccurate. Actual memory usage is around 300-500MB.

Suggestion - do not print memory usage at all.

Hence the PR.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@koresar
Copy link
Author

koresar commented Apr 11, 2019

The build is broken because npm install --package-lock-only && npm audit --audit-level=moderate returns exit 1 - one vulnerable package found.

I'm afraid fixing that myself. Sorry.

@pimlie
Copy link

pimlie commented Apr 11, 2019

Instead of removing the functionality we could also move it. Eg add a method showMemoryUsage which uses a normal consola.info log to print the memory stats, then that method could be run eg after the build has finished

@koresar
Copy link
Author

koresar commented Apr 11, 2019

I'm happy if this PR is closed.

@pimlie
Copy link

pimlie commented Apr 11, 2019

@koresar I was hoping that you would be willing to change this PR to accommodate my suggestion 😄

Should be a nice first contribution which will be usefull to all nuxt users (and that doesnt happen often for first time contributors!)

@koresar
Copy link
Author

koresar commented Apr 11, 2019

Sorry mate. I'd love to contribute. But that sounds complicated even though it is easy for pros like yourself.

Maybe to improve the situation we merge this PR and create a new "good first issue" for volunteers specifically to implement your suggestion?

@pimlie
Copy link

pimlie commented Apr 11, 2019

I was trying to change it just now but it appears you made your PR from the defunct next branch. I understand the confusion, but it should've been the dev branch. So will close this in favour of a new PR.

Thanks for your work tough!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants