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

IO-724 FileUtils#deleteDirectory(File) exception Javadoc inaccurate update #245

Merged
merged 3 commits into from
Jun 22, 2021

Conversation

liran2000
Copy link
Contributor

@liran2000 liran2000 commented Jun 13, 2021

FileUtils.deleteDirectory javadoc is inaccurate for nonexistent directory.

Change for returned exception javadoc, as the conditions at the method implementation do not match the javadoc.

Method implementation does not throws the IllegalArgumentException from the javadoc if directory does not exist:
if (!directory.exists()) {
return;
}

This one line code proves it:
FileUtils.deleteDirectory(new File("nonexistingdir"));

FileUtils.deleteDirectory javadoc is inaccurate for nonexistent directory
@liran2000 liran2000 changed the title IO-724 deleteDirectory exception javadoc update IO-724 deleteDirectory exception javadoc inaccurate update Jun 13, 2021
@XenoAmess
Copy link
Contributor

Thinng about this will be better when after we apply JSR305

If my memory be correct, there be a branch adding these annotations to suggest Nullable or NotNull.

How's it going @garydgregory ?

Remove NullPointerException from javadoc as future annotation may comes to place
@liran2000
Copy link
Contributor Author

liran2000 commented Jun 14, 2021

Thinng about this will be better when after we apply JSR305

If my memory be correct, there be a branch adding these annotations to suggest Nullable or NotNull.

The most important here is to remove the "IllegalArgumentException" if directory does not exist from the javadoc, which does not match the implementation condition: if (!directory.exists()) { return;}

Regarding Nullable/NotNull, got it, removed this addition for now.

@coveralls
Copy link

coveralls commented Jun 14, 2021

Coverage Status

Coverage remained the same at 89.25% when pulling eb4d3c8 on liran2000:IO-724-javadoc into ff02373 on apache:master.

@garydgregory
Copy link
Member

garydgregory commented Jun 15, 2021

Thinng about this will be better when after we apply JSR305

If my memory be correct, there be a branch adding these annotations to suggest Nullable or NotNull.

How's it going @garydgregory ?

Fixing Javadoc here is good. JSR305/8 is a different story, which we are not doing for now.

@liran2000
Copy link
Contributor Author

@XenoAmess @garydgregory thank you, can you then please review and merge this PR ?

@garydgregory
Copy link
Member

@XenoAmess @garydgregory thank you, can you then please review and merge this PR ?

I should be able to look at this later today, AFK ATM.

@liran2000
Copy link
Contributor Author

@XenoAmess @garydgregory thank you, can you then please review and merge this PR ?

I should be able to look at this later today, AFK ATM.

@garydgregory thanks, will wait for your review and merge.

@garydgregory garydgregory merged commit 5c1a230 into apache:master Jun 22, 2021
@garydgregory garydgregory changed the title IO-724 deleteDirectory exception javadoc inaccurate update IO-724 FileUtils#deleteDirectory(File) exception Javadoc inaccurate update Jun 22, 2021
@liran2000 liran2000 deleted the IO-724-javadoc branch June 22, 2021 12:50
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.

4 participants