Skip to content

feat: make it harder to skip graceful shutdown accidentally#1327

Merged
spikecurtis merged 2 commits into
mainfrom
spike/631-dont-allow-double-ctrl-c
May 6, 2022
Merged

feat: make it harder to skip graceful shutdown accidentally#1327
spikecurtis merged 2 commits into
mainfrom
spike/631-dont-allow-double-ctrl-c

Conversation

@spikecurtis

Copy link
Copy Markdown
Contributor

Signed-off-by: Spike Curtis spike@coder.com

Makes it harder to accidentally skip graceful shutdown (which, in --dev mode includes cleaning up workspaces) by not allowing a second ctrl+C to kill the program. If you really want to kill graceful shutdown, you can use ctrl+\ (SIGQUIT).

Subtasks

  • added a test for feature

Fixes #631

Signed-off-by: Spike Curtis <spike@coder.com>
@spikecurtis spikecurtis requested review from bpmct and kylecarbs May 6, 2022 17:49

@kylecarbs kylecarbs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@codecov

codecov Bot commented May 6, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1327 (ff7e74c) into main (57bb108) will increase coverage by 8.36%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1327      +/-   ##
==========================================
+ Coverage   58.09%   66.45%   +8.36%     
==========================================
  Files         282      282              
  Lines       18521    18522       +1     
  Branches      231      231              
==========================================
+ Hits        10759    12309    +1550     
+ Misses       6601     4949    -1652     
- Partials     1161     1264     +103     
Flag Coverage Δ
unittest-go-macos-latest 53.68% <100.00%> (+<0.01%) ⬆️
unittest-go-postgres- 65.12% <100.00%> (?)
unittest-go-ubuntu-latest 56.10% <100.00%> (+0.05%) ⬆️
unittest-go-windows-2022 52.00% <0.00%> (-0.02%) ⬇️
unittest-js 73.14% <ø> (ø)
Impacted Files Coverage Δ
cli/server.go 57.51% <100.00%> (+3.70%) ⬆️
codersdk/provisionerdaemons.go 61.97% <0.00%> (-5.64%) ⬇️
provisioner/echo/serve.go 56.80% <0.00%> (ø)
agent/agent.go 66.94% <0.00%> (+0.41%) ⬆️
coderd/provisionerdaemons.go 62.98% <0.00%> (+0.50%) ⬆️
coderd/workspaceagents.go 56.04% <0.00%> (+0.69%) ⬆️
provisionerd/provisionerd.go 77.64% <0.00%> (+1.20%) ⬆️
coderd/templates.go 62.29% <0.00%> (+1.63%) ⬆️
peer/conn.go 80.34% <0.00%> (+2.94%) ⬆️
... and 7 more

Continue to review full report at Codecov.

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

@bpmct bpmct left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is awesome

Signed-off-by: Spike Curtis <spike@coder.com>
@spikecurtis spikecurtis merged commit ad8d9dd into main May 6, 2022
@spikecurtis spikecurtis deleted the spike/631-dont-allow-double-ctrl-c branch May 6, 2022 20:45
@mafredri

mafredri commented May 11, 2022

Copy link
Copy Markdown
Member

TIL about Ctrl+\ too, very neat.

I realize this PR is merged but wanted to give a heads up about non-English keyboard users who'd have to hit Ctrl+Shift+Alt+7 (at least on macOS). Some may end up unable to escape (queue joke about VIM).

Edit: Actually that combination doesn't even work (I just tested), you get \ with Shift+Alt+7 but add Ctrl and all you get is 7.

@misskniss misskniss added this to the V2 Beta milestone May 15, 2022
kylecarbs pushed a commit that referenced this pull request Jun 10, 2022
* feat: make it harder to skip graceful shutdown accidentally

Signed-off-by: Spike Curtis <spike@coder.com>

* fixup: don't use unbuffered signal channel

Signed-off-by: Spike Curtis <spike@coder.com>
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.

Add double confirmation for SIGINT on coder --dev

5 participants