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 use of System.exit() calls from codebase #20

Closed
lewismc opened this issue Feb 23, 2017 · 4 comments
Closed

Remove use of System.exit() calls from codebase #20

lewismc opened this issue Feb 23, 2017 · 4 comments

Comments

@lewismc
Copy link

lewismc commented Feb 23, 2017

I'm currently working on building openie into Apache Any23 via apache/any23#33, a major issue I am having however is with Unit testing the code I'm attempting to implement.
The is due to the Maven surefire plugin failing whenever it encounters System.exit().
I've scanned the openie codebase and there appears to be a few places where this is the case.
I am not 100% sure that this stems from openie, however I will begin by submitting the PR here to narrow the issue down.

@lewismc
Copy link
Author

lewismc commented Feb 23, 2017

@Yongyao CC for context

@jkinkead
Copy link
Contributor

The Relnoun case is definitely an error; that should be throwing an exception or similar.

The Nesty case is in a main method, so it seems appropriate.

@lewismc
Copy link
Author

lewismc commented Feb 23, 2017

OK thanks @jkinkead I'm working on a PR and testing locally to see if this resolves my issue.

The Nesty case is in a main method, so it seems appropriate.

Two things spring to mind

  • is there any reason for returning the non-zero code?
  • Seeing as we are within main this would be all the more reason to not use System.exit(). Main will terminate after the exception message has been thrown.

@aimichal
Copy link
Contributor

aimichal commented Jan 3, 2019

We're not actively working on making changes to this codebase at the moment. If you have a suggestion for what to change, a PR would be welcome. (I agree that deeply nested library code shouldn't make System.exit() calls.)

Closing this issue now as part of cleanup, but please re-open if there's more to discuss.

@aimichal aimichal closed this as completed Jan 3, 2019
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

No branches or pull requests

3 participants