Skip to content
This repository was archived by the owner on May 28, 2018. It is now read-only.

Conversation

@Stephan202
Copy link
Contributor

This is a follow up of #132. In that ticket @cguillot pointed out that as a side effect of the solution to #132, the parent parameter to JarFileScanner may no longer have a trailing slash. Since the parent parameter is clearly meant to be interpreted as a path component, I agree with @cguillot that this is a regression worth fixing.

This PR restores support for trailing slashes and adds two unit tests to prove it. As a side effect the JarFileScanner logic actually became quite a bit simpler.

As it stands, this PR introduces a small piece of conditional logic in the JarFileScanner constructor. If you prefer, I can also push this down into #hasNext() (at probably negligible performance cost).

@jerseyrobot
Copy link
Contributor

Can one of the admins verify this patch?

@mgajdos
Copy link
Contributor

mgajdos commented Feb 22, 2015

Jenkins, test this patch.

@mgajdos mgajdos self-assigned this Feb 22, 2015
@Stephan202
Copy link
Contributor Author

Hmm, the Jenkins build failed due to a test timeout. Earlier builds failed for the same reason, so I assume this is unrelated. (Locally the whole build ran just fine.)

@mgajdos
Copy link
Contributor

mgajdos commented Feb 22, 2015

Yeah, not caused by your change. This test sometimes fails on CloudBees. I'll verify this manually.

@mgajdos mgajdos added this to the 2.17 milestone Feb 22, 2015
@mgajdos
Copy link
Contributor

mgajdos commented Feb 23, 2015

Verified manually.

mgajdos added a commit that referenced this pull request Feb 26, 2015
…ailing-slash-fix

Restore JarFileScanner support for parents with a trailing slash
@mgajdos mgajdos merged commit 41678fc into javaee:master Feb 26, 2015
@mgajdos
Copy link
Contributor

mgajdos commented Feb 26, 2015

Thanks for fixing the issue.

@Stephan202 Stephan202 deleted the jarfilescanner-parent-with-trailing-slash-fix branch February 26, 2015 17:06
@Stephan202
Copy link
Contributor Author

No problem, you're welcome. Thanks for merging it :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants