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

feat: graceful exit terminal #3075

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

feat: graceful exit terminal #3075

wants to merge 15 commits into from

Conversation

rishabh3112
Copy link
Member

What kind of change does this PR introduce?
feature

Did you add tests for your changes?
WIP
If relevant, did you update the documentation?

Summary
Added code for graceful exit.
For reference: https://github.com/webpack/webpack-dev-server/blob/75999bb9bb8803de633fcb037405f45a5bf7d029/lib/Server.js#L1808-L1838

Does this PR introduce a breaking change?
Yup can be for tools that consume webpack-cli's output.

Other information
/cc @alexander-akait

@rishabh3112
Copy link
Member Author

@alexander-akait should we add flag to enable this behavior as dev server does?
If we keep it by default, it can be a breaking change for tools that rely on webpack-cli's output.

@codecov
Copy link

codecov bot commented Jan 9, 2022

Codecov Report

Attention: Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.48%. Comparing base (d242dea) to head (9ea48e7).
Report is 908 commits behind head on master.

Files with missing lines Patch % Lines
packages/webpack-cli/src/webpack-cli.ts 90.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3075      +/-   ##
==========================================
- Coverage   91.50%   91.48%   -0.03%     
==========================================
  Files          23       23              
  Lines        1719     1738      +19     
  Branches      519      524       +5     
==========================================
+ Hits         1573     1590      +17     
- Misses        146      148       +2     
Files with missing lines Coverage Δ
packages/webpack-cli/src/webpack-cli.ts 93.96% <90.00%> (-0.09%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d242dea...9ea48e7. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Looks good, let's add couple tests - just create custom plugin with infinity waiting and send SIGINT signals to close it

@rishabh3112 rishabh3112 marked this pull request as ready for review February 25, 2022 06:50
@rishabh3112 rishabh3112 requested a review from a team as a code owner February 25, 2022 06:50
@rishabh3112
Copy link
Member Author

/cc @alexander-akait

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

could you rebase?

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

No activity, closing.

@alexander-akait
Copy link
Member

@evenstensberg let's keep, it is a bug on our side

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