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

Refactor + display changes #7403

Merged
merged 16 commits into from
Apr 24, 2024
Merged

Refactor + display changes #7403

merged 16 commits into from
Apr 24, 2024

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Apr 23, 2024

A mix of refactoring that fell out from the new display changes and some additional behavior.

This PR is separated into commits that are smaller logical chunks of work. I'll call out the most important changes here:

CLI

  • Created lib/cli/ dir for files that are CLI specific. This should have more files added to it in the future as we separate CLI behavior from new Npm() behavior.
    • Moved some behavior to CLI files such as update-notifier, nonDashArgs, and graceful-fs
    • Also moved some things out of exit-handler such as logging about the log file error and timing file. These required getting a bunch of state from npm that can now be kept internal
  • npm.load() has the ability to exit early, this is only used for npm --version right now but this speeds up the command from having to create logfiles and caches it will never use
  • Make some properties of npm, timers, logfile private since they are no longer needed outside of themselves

File structure

  • Made lib/utils/ completely flat and moved some lib/workspaces into lib/utils/
  • Renamed base-command to base-cmd to match other commands

Display

  • Use proc-log.META for forcing logs and flushing output
  • Make display responsible for creating chalk instances
  • Don't hide all output on silent. This uses a hack to hide spawn banner output to match previous behavior now
  • Remove a bunch of timers and logs that were superflous

Log file

  • Don't write timing logs to debug log file unless --timing is set

@lukekarrys lukekarrys force-pushed the lk/move-files branch 4 times, most recently from 49e3272 to f56447d Compare April 24, 2024 17:59
@lukekarrys lukekarrys changed the title Refactor WIP Refactor + display changes Apr 24, 2024
@lukekarrys lukekarrys marked this pull request as ready for review April 24, 2024 18:35
@lukekarrys lukekarrys requested a review from a team as a code owner April 24, 2024 18:35
Copy link
Member

@wraithgar wraithgar left a comment

Choose a reason for hiding this comment

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

Just one "are we sure we need this?" question on a test, not a show stopper.

@lukekarrys lukekarrys merged commit 7f4e667 into latest Apr 24, 2024
49 checks passed
@lukekarrys lukekarrys deleted the lk/move-files branch April 24, 2024 21:33
@github-actions github-actions bot mentioned this pull request Apr 24, 2024
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