-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Javadocs and minor refactoring for Nearby package #2188
Conversation
* This solution is an hotfix for overrite issue came back on 2.8.0 version. What I did is checking the extension, and if it is null, adding .jpg suffix. Because commons files always have suffixes, and we should compare file names after adding suffixes. Othervise overrides are possible. * Check if file title includes an extension already, by checking if is there any dot in it. * Fix logic error * Add uncovered tests * Remove unecessary line breaks * Make Javadocs more explicit
* Versioning for v2.8.2 * Changelog for v2.8.2
# Conflicts: # CHANGELOG.md # app/build.gradle # app/src/main/java/fr/free/nrw/commons/CommonsApplication.java
…should not be in all caps
…idy-nearby # Conflicts: # app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java
Codecov Report
@@ Coverage Diff @@
## master #2188 +/- ##
=========================================
- Coverage 5.72% 5.71% -0.01%
=========================================
Files 231 231
Lines 11587 11598 +11
Branches 1078 1079 +1
=========================================
Hits 663 663
- Misses 10869 10880 +11
Partials 55 55
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy with everything apart from DirectUpload which seemed messy with all those nested if statements, so I pushed f25b267.
Tested 2.9.0-debug-pr-2188~f25b2679c
on Galaxy Nexus (emulator)
at API level 28
Tested 2.9.0-debug-pr-2188~f25b2679c
on Samsung Galaxy S6
at API level 25
Everything I tried worked (apart from known issues), and on the emulator I can see the list of nearby places which looks correct. Because of #2167 and #2168 I cannot comment on the nearby map itself as don't have any devices it works on.
Given that I've now worked on it I'd ideally like someone else to merge.
Thanks @domdomegg . You're right about the nested if statements, the current code does look better. :) |
Final PR for #1828 . Added Javadocs, fixed lint issues and optimized imports for several classes in Nearby package. Did a minor refactor of NearbyPlaces.java and DirectUpload.java to make it more readable.
Tested on Nexus S running API 27.