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.

Sign up for GitHub

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

Jump to bottom

Fix exit code when Ctrl+C'ing dev server #127

Merged
merged 1 commit into from
Mar 10, 2023
Merged

Fix exit code when Ctrl+C'ing dev server #127

merged 1 commit into from
Mar 10, 2023

Conversation

feedmeapples
Copy link
Contributor

What was changed

Made Ctrl+C exit code 0 instead of 1 when running dev server

Why?

proper RC

address #43

Checklist

  1. Closes

  2. How was this tested:

temporal server start-dev

Hit Ctrl+C

echo $?
0
  1. Any docs updates needed?

bergundy
bergundy reviewed Feb 24, 2023
View reviewed changes
Copy link
Member

@bergundy bergundy left a comment

Choose a reason for hiding this comment

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

Please test for both dev server and non dev server

@feedmeapples
Copy link
Contributor Author

feedmeapples commented Feb 24, 2023 •
edited
Loading

Please test for both dev server and non dev server

already checked temporalio/temporal server and it properly returns RC 0. They use urfave/cli's default error handler cli.HandleExitCoder(err) which we now also use after logging custom messages

looking into the test failures

@bergundy
Copy link
Member

I understand but it would be awesome if we had automated tests for this

@feedmeapples feedmeapples force-pushed the fix-exit-code branch 3 times, most recently from c64202b to 2a2bb60 Compare March 9, 2023 05:20
@feedmeapples feedmeapples changed the title Fix RC 1 on dev server Ctrl+C Fix exit code when Ctrl+C'ing dev server Mar 10, 2023
@feedmeapples feedmeapples merged commit 000d2da into main Mar 10, 2023
@feedmeapples feedmeapples deleted the fix-exit-code branch March 10, 2023 06:43
@feedmeapples feedmeapples linked an issue Mar 10, 2023 that may be closed by this pull request
[Bug] Hitting Ctrl-C causes dev server to exit with RC 1 and print an error #43
Closed
@feedmeapples feedmeapples mentioned this pull request Mar 10, 2023
Merged
sevein added a commit to sevein/sdk-go that referenced this pull request Mar 27, 2023
@sevein
Fix use of defer in DevServer tests
a547f03 
DevServer tests are not passing because `server.Stop()` was evaluated too early
as function parameters in combination with `defer`.

I'd be introducing a function literal, but instead I'm ignoring the err returned
until temporalio/cli#127 is completely addressed.
@cretz cretz mentioned this pull request Mar 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robholland robholland robholland approved these changes

@bergundy bergundy Awaiting requested review from bergundy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Hitting Ctrl-C causes dev server to exit with RC 1 and print an error
3 participants
@feedmeapples @bergundy @robholland