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

Make Java common used exceptions public for users. #60

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

psionic12
Copy link

Some Java codes take an exception as a parameter (e.g. public void callFailed(Call call, IOException ioe) in okhttp)

Make JXXExceptions public to save time.

Some Java codes take an exception as a paramter (e.g.
`public void callFailed(Call call, IOException ioe)` in okhttp)
Make JXXExceptions public to save time.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 22, 2021
@facebook-github-bot
Copy link
Contributor

@passy has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@dreiss
Copy link
Contributor

dreiss commented Jul 21, 2021

Does it make sense to add java.lang.Exception, for completeness, and have most of these extend that, rather than Throwable?

Does it make sense to move these to Exceptions.h?

ArrayIndexOutOfBounds exception should extend RuntimeException, rather than throwable.

@psionic12
Copy link
Author

While this pr are not intended to change fbjni too much, just expose the predefined exception for an easy use. What you mentioned sound reasonable but need a fully discussion with other fbjni members, I guess.

@dreiss
Copy link
Contributor

dreiss commented Jul 22, 2021

just expose the predefined exception for an easy use

The issue here is that there are some deficiencies in the existing implementations that didn't cause any problems with the specific ways they were being used, but that should be cleaned up before we make them available as general-purpose utilities for consumers.

need a fully discussion with other fbjni members

These suggestions were made internally by Marc Horowitz, and I agree with them. That's more than enough fbjni members to approve this change. :)

@facebook-github-bot
Copy link
Contributor

@psionic12 has updated the pull request. You must reimport the pull request before landing.

@psionic12
Copy link
Author

@dreiss Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants