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

refactor search #4368

Closed
wants to merge 2 commits into from
Closed

Conversation

AlexNi245
Copy link
Contributor

@AlexNi245 AlexNi245 commented Aug 21, 2019

this pr referees to #2723
the goal here is just to refactor the existing search by moving the search logic from OCFileListFragment into a own class.
i opened a pr at that early to have a good place for further discussions.
@AndyScherzinger @tobiasKaminsky @jancborchardt

… search capabilities from OCFileListFragment to NamedSearchResultFragment without touching the FileDisplayActivity
@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/10628.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

289

Lint

TypemasterPR
Warnings5959
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings24
Correctness Warnings69
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings110
Security Warnings46
Dodgy code Warnings138
Total412

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings24
Correctness Warnings69
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings110
Security Warnings46
Dodgy code Warnings138
Total412

@codecov
Copy link

codecov bot commented Aug 22, 2019

Codecov Report

Merging #4368 into master will increase coverage by 0.02%.
The diff coverage is 24.17%.

@@             Coverage Diff              @@
##             master    #4368      +/-   ##
============================================
+ Coverage     16.84%   16.87%   +0.02%     
- Complexity        1        3       +2     
============================================
  Files           358      360       +2     
  Lines         31805    31823      +18     
  Branches       4491     4493       +2     
============================================
+ Hits           5358     5370      +12     
- Misses        25551    25559       +8     
+ Partials        896      894       -2
Impacted Files Coverage Δ Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...android/ui/fragment/NamedSearchResultFragment.java 100% <100%> (ø) 0 <0> (?)
...cloud/android/ui/activity/FileDisplayActivity.java 20.03% <23.75%> (ø) 0 <0> (ø) ⬇️
...va/com/owncloud/android/ui/SquareLinearLayout.java 0% <0%> (-50%) 0% <0%> (ø)
.../third_parties/daveKoeller/AlphanumComparator.java 82.14% <0%> (-1.2%) 0% <0%> (ø)
...owncloud/android/ui/adapter/OCFileListAdapter.java 28.59% <0%> (-1.16%) 0% <0%> (ø)
...loud/android/datamodel/ThumbnailsCacheManager.java 35.76% <0%> (-0.45%) 0% <0%> (ø)
...ncloud/android/ui/fragment/OCFileListFragment.java 25.44% <0%> (-0.28%) 0% <0%> (ø)
...m/owncloud/android/ui/activity/DrawerActivity.java 44.53% <0%> (-0.21%) 0% <0%> (ø)
...a/com/owncloud/android/jobs/ContactsBackupJob.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
... and 14 more

@jancborchardt
Copy link
Member

@AlexNi245 good stuff! :) As soon as there’s anything visible, what would be very helpful for me to review is screenshots or maybe gifs. :) (I can recommend Peek: https://github.com/phw/peek )

@AlexNi245
Copy link
Contributor Author

@jancborchardt thank you for your recommendation. I will mention you if there is anything to see. This week i have a lot to do so i hope i find some time at weekend. Hope this issue is non time critical :)

@jancborchardt
Copy link
Member

No rush, it’s not time critical. :) Thanks a lot for picking it up and I’m looking forward to it!

@tobiasKaminsky
Copy link
Member

I think this can be closed as we have unified search: #7568
If I am wrong, please re-open.

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

Successfully merging this pull request may close these issues.

None yet

4 participants