Skip to content

New media detection#2406

Merged
tobiasKaminsky merged 20 commits into
masterfrom
new-media-detection
Jul 13, 2018
Merged

New media detection#2406
tobiasKaminsky merged 20 commits into
masterfrom
new-media-detection

Conversation

@mario

@mario mario commented Mar 27, 2018

Copy link
Copy Markdown
Contributor

Fixes #1447

Detects new media folder and notifies users about it.

@mario

mario commented Mar 27, 2018

Copy link
Copy Markdown
Contributor Author

This is now ready for review folks! @AndyScherzinger @tobiasKaminsky

@nextcloud nextcloud deleted a comment Mar 27, 2018
@nextcloud nextcloud deleted a comment Mar 27, 2018
@nextcloud nextcloud deleted a comment Mar 27, 2018
@mario

mario commented Mar 27, 2018

Copy link
Copy Markdown
Contributor Author

Ummm so I checked lint report, but don't see what new warnings are?

@tobiasKaminsky

Copy link
Copy Markdown
Member

You have to compare it with master.html which is in the same directory.
There is a lint tool to use current master as baseline, which then should only show the difference, but this does not work, so we have to compare it manually :/

@nextcloud nextcloud deleted a comment Mar 29, 2018
@nextcloud nextcloud deleted a comment Mar 29, 2018
@nextcloud nextcloud deleted a comment Mar 29, 2018
@mario

mario commented Mar 29, 2018

Copy link
Copy Markdown
Contributor Author

Looks ok now @tobiasKaminsky. Please test and review.

@nextcloud nextcloud deleted a comment Apr 3, 2018
@AndyScherzinger

Copy link
Copy Markdown
Member

I tested it installing OpenCamera and that didn't completely work for me.

  • Works: Detection of new Folder 👍
  • Failed: Directly configuring the new folder via the notification :(

The configuration is active while the pictures taken after don't get detected / uploaded.

@mario

mario commented Apr 4, 2018

Copy link
Copy Markdown
Contributor Author

@AndyScherzinger could you look into NCContentObserverJob and see what syncedFolderProvider.countEnabledSyncedFolders() returns there?

@mario

mario commented Apr 11, 2018

Copy link
Copy Markdown
Contributor Author

@AndyScherzinger @tobiasKaminsky fixed for real this time. Please rebase and test. Thanks!!!

@nextcloud nextcloud deleted a comment Apr 11, 2018
@AndyScherzinger

Copy link
Copy Markdown
Member

Can't test right now (time-wise) but did the rebase :)

@nextcloud nextcloud deleted a comment Apr 11, 2018
@nextcloud nextcloud deleted a comment Apr 11, 2018
@tobiasKaminsky tobiasKaminsky mentioned this pull request Apr 13, 2018
6 tasks
@mario

mario commented Apr 22, 2018

Copy link
Copy Markdown
Contributor Author

Anyone tested this?

@nextcloud nextcloud deleted a comment Apr 22, 2018
@AndyScherzinger AndyScherzinger force-pushed the new-media-detection branch 4 times, most recently from fa94bd9 to eb174a9 Compare June 24, 2018 09:07
@AndyScherzinger

Copy link
Copy Markdown
Member

Imho now ready to review @tobiasKaminsky

I fixed:

  • Directly configuring the new folder via the notification (now calculation the index in the list)
  • Fixing the navigation issues described in New media detection #2406 (comment)
  • Fixing the codacy issues that now came up due to the more restrictive checks due to the activation of pmd

@AndyScherzinger

AndyScherzinger commented Jul 5, 2018

Copy link
Copy Markdown
Member

👍 approved from my side while I of course added / rewrote some code to fix the issues

so pinging @tobiasKaminsky and @ardevd for second review :)

Approved with PullApprove

@mario

mario commented Jul 11, 2018

Copy link
Copy Markdown
Contributor Author

👍

cc @AndyScherzinger @tobiasKaminsky

Approved with PullApprove

@nextcloud-android-bot

Copy link
Copy Markdown
Collaborator

Lint

This 95 warnings
Master 95 warnings

FindBugs (new)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings166
Experimental Warnings4
Internationalization Warnings12
Malicious code vulnerability Warnings9
Multithreaded correctness Warnings9
Performance Warnings159
Security Warnings163
Dodgy code Warnings207
Total764

FindBugs (master)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings166
Experimental Warnings4
Internationalization Warnings12
Malicious code vulnerability Warnings9
Multithreaded correctness Warnings9
Performance Warnings158
Security Warnings163
Dodgy code Warnings209
Total765

@tobiasKaminsky

tobiasKaminsky commented Jul 13, 2018

Copy link
Copy Markdown
Member

👍

Approved with PullApprove

@jasonbayton

Copy link
Copy Markdown
Member

Oh this is exciting! Thanks folks!

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.

5 participants