search

sonatype-nexus-community / nexus-repository-apk

Eclipse Public License 1.0
42 stars 24 forks source link

Update ApkPathUtils.java #5

Closed adrianp-sti closed 4 years ago

adrianp-sti commented 4 years ago

Fix for package names with period in the main part of the name

(brief, plain english overview of your changes here)

This pull request makes the following changes:

(If there are changes to the UI, please include a screenshot so we know what to look for!)

(If there are changes to user behavior in general, please make sure to update the docs, as well)

It relates to the following issue #s:

cc @DarthHater @bhamail

sonatypecla[bot] commented 4 years ago

Thanks for the contribution! Before we can merge this, we need @adrianp-sti to sign the Sonatype Contributor License Agreement.

DarthHater commented 4 years ago

@adrianp-sti couple tiny observations:

I think @allenhsieh will be popping in here, he might be able to help you out if you'd like!

bhamail commented 4 years ago

More deets in Issue #4

adrianp-sti commented 4 years ago

Hi Allen,

Thanks for the offer of help. Specifically it was lua5.3-libs-5.3.5-r2.apk that was causing the issue. To be honest I think it'll be any package that has a period in the main part of the package name but the Lua one was the first example I came across. I ended up just writing a small test based on the name() method in ApkPathUtils.java to see if the fix worked and didn't adversely affect the parsing of any other APK names.

adrian.

On Wed, 13 Nov 2019 at 17:09, Allen Hsieh notifications@github.com wrote:

Hi @adrianp-sti https://github.com/adrianp-sti, I can help with getting some unit/integration tests up today. Was there a specific package that was causing you issues that I can test against?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sonatype-nexus-community/nexus-repository-apk/pull/5?email_source=notifications&email_token=ACHXPJKTL2B54ELQT55R4ALQTQYDBA5CNFSM4JL2IFSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED63NLA#issuecomment-553498284, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHXPJJSIXNNMI57QUHPGJDQTQYDBANCNFSM4JL2IFSA .

allenhsieh commented 4 years ago

Hey @adrianp-sti thanks for your reply! I deleted my initial comment because it was graciously pointed out to me that @bhamail linked to the issue, and the issue named the package in question.

I forked off your fork and just finish writing some unit and integration tests, it should be good to squash and merge: https://github.com/adrianp-sti/nexus-repository-apk/pull/1

adrianp-sti commented 4 years ago

I've found a few more use cases which I'll comment on in #4 so until there's a fix for all the new issues I'll close this and submit a new pull-up when ready.