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

VFS-189: fix issue by checking if base filename is null before using it #10

Closed
wants to merge 1 commit into from

Conversation

kinow
Copy link
Member

@kinow kinow commented Feb 1, 2016

Checks if the base filename is null before using, preventing a NPE. More on the rationale for that in the comment entered into the issue for this pull request.

@PascalSchumacher
Copy link
Contributor

What about merging this?

@eepstein
Copy link

eepstein commented Aug 8, 2017

I thought this is the approved means for the community to submit patches to the VFS code, but the above pull request has been open for 1.5 years and no one with write access to the repo has responded. Is anyone with write access watching this repo ?

@kinow
Copy link
Member Author

kinow commented Aug 9, 2017

@eepstein

I thought this is the approved means for the community to submit patches to the VFS code, but the above pull request has been open for 1.5 years and no one with write access to the repo has responded.

My mistake. I also have writing permissions, but I submit pull requests sometimes to wait for feedback and review. I forgot about this issue, and thus didn't ping the mailing list to get someone to review. Nor did I merge it myself.

Merging it now.

Is anyone with write access watching this repo ?

I am, there are others with write access that I believe are also watching this repo. But even without watching, there is some tasks in the Apache infrastructure that mirror notifications here to JIRA and/or mailing lists.

Thanks
Bruno

@kinow
Copy link
Member Author

kinow commented Aug 9, 2017

Merged in SVN revision 1804480. Forgot to include the "This closes #10" so that GitHub would close this PR. Closing now.

Some tests failed for me, but mvn clean test -Dtest=DefaultFileSystemManagerTest#testResolveFileNameNull -Dsurefire.failIfNoSpecifiedTests=false showed that this change is not the issue.

@kinow kinow closed this Aug 9, 2017
@eepstein
Copy link

eepstein commented Aug 11, 2017 via email

@PascalSchumacher
Copy link
Contributor

@kinow Thanks!

I fixed some test failures with 1655496

Maybe you also get the https.test.GetContentInfoFunctionalTest failure which occurs on travis-ci with java 8? (I have no idea what the correct fix for this is (maybe it is a bug?), so I created: https://issues.apache.org/jira/browse/VFS-641

@kinow
Copy link
Member Author

kinow commented Aug 13, 2017

Thanks @PascalSchumacher ! I noticed these tests failing, do you think this failure is related to this pull request? My initial impression was that it was failing for some other reason. I believe I got the same failure.

I have Ubuntu and Windows 10 at work, so can help debugging/troubleshooting this issue during the next days. Though I will be probably busy using any spare time to read docs to act as release manager for csv...

@PascalSchumacher
Copy link
Contributor

PascalSchumacher commented Aug 13, 2017

I do not know the vfs code base, but my guess is that the https.test.GetContentInfoFunctionalTest failure is not caused by your pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants