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

Fix #2751: javalib Files#delete now throws informative DirectoryNotEmptyException #2753

Merged

Conversation

LeeTibbert
Copy link
Contributor

@LeeTibbert LeeTibbert commented Jul 25, 2022

For the 0.5.0 train This is the 0.5.0 version of PR #2754

The javalib Files#delete method now throws the DirectoryNotEmptyException when attempting
to delete a directory which is not empty. This Exception is a subclass of IOException.

The Java 8 documentation describes it as an "optional" specialization of IOException.
A more detailed Exception gives the end user a better clue.

This PR is the result of teamwork: I supplied the bugs & armanbilge supplied the
motivation and collaboration.

@armanbilge
Copy link
Member

armanbilge commented Jul 25, 2022

You did it!! Thank you so much!! Since it's okay to target to 0.4.x, would you mind changing the base branch in the PR so that we don't forget? Edit: also so that we run CI against the appropriate branch.

@LeeTibbert
Copy link
Contributor Author

It is not pretty but it is, I believe, fit for purpose. Windows throws the same Exception, so there
is no off-putting quirk there.

re: Since it's okay to target to 0.4.x, would you mind changing the base branch in the PR so that we don't forget?
Is there git or GitHub magic to do that or do I have to start over, branch off of 0.4.5, copy files around, and
submit another PR? Will 0.4.n PRs get merged into the 0.5.0 stream, I do I need to maintain two PRs?

so that we run CI against the appropriate branch.
Now that would make too much sense. Does multi-arch work better in 0.4.n land?

@LeeTibbert
Copy link
Contributor Author

This is ready for review. There is one of the usual failing multi-arch tests and
one failing Linux runtime test. The latter appears to have nothing to do with
this PR.

@LeeTibbert LeeTibbert added component:javalib has pr There is a pending PR that addresses this issue and removed component:javalib has pr There is a pending PR that addresses this issue labels Jul 27, 2022
@WojciechMazur WojciechMazur merged commit 62e6302 into scala-native:main Aug 4, 2022
WojciechMazur pushed a commit to WojciechMazur/scala-native that referenced this pull request Aug 29, 2022
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.

None yet

3 participants