Skip to content

Conversation

@tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Oct 11, 2018

Needs nextcloud/android-library#73 and nextcloud/activity#300

Previously we tried to find a referenced file in our database and then generated a thumbnail out of it.
Now we rely on the server to get a preview url. This way we can have thumbnails for files we never had in our database before.

TODO

  • add backwards compatibility

Signed-off-by: tobiasKaminsky [email protected]

@tobiasKaminsky
Copy link
Member Author

Damn, I totally forgot the backward compatibility :-/

@nextcloud nextcloud deleted a comment Oct 12, 2018
@tobiasKaminsky tobiasKaminsky force-pushed the activityPreview branch 4 times, most recently from 0823cd1 to 181d7f6 Compare December 10, 2018 15:19
@AndyScherzinger
Copy link
Member

@tobiasKaminsky the lib change say compatible with Nc14. Is that what you meant with backwards compatible, because there is still also Nc13 maybe 12?

@tobiasKaminsky
Copy link
Member Author

I am using old thumbnail system on <15 and new on >=15:
https://github.com/nextcloud/android/pull/3130/files#diff-521d2e61330253bb0889bddc78c08d0dR204

It depends on activity app version, but I would like to bind it to a NC version, so that we can remove it on NC19 :-)

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Dec 13, 2018

👍

Approved with PullApprove

1 similar comment
@mario
Copy link
Contributor

mario commented Dec 13, 2018

👍

Approved with PullApprove

@nextcloud-android-bot
Copy link
Collaborator

Lint

TypemasterPR
Warnings8383
Errors00

FindBugs (new)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings159
Internationalization Warnings15
Malicious code vulnerability Warnings11
Multithreaded correctness Warnings9
Performance Warnings121
Security Warnings66
Dodgy code Warnings134
Total550

FindBugs (master)

Warning TypeNumber
Bad practice Warnings35
Correctness Warnings159
Internationalization Warnings15
Malicious code vulnerability Warnings11
Multithreaded correctness Warnings9
Performance Warnings121
Security Warnings66
Dodgy code Warnings135
Total551

@tobiasKaminsky tobiasKaminsky merged commit 824a91a into master Dec 14, 2018
@tobiasKaminsky tobiasKaminsky deleted the activityPreview branch December 14, 2018 08:01
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Dec 14, 2018

@tobiasKaminsky fancy taking a look? I am on the train right now so I can't :/

EDIT: drone was just using the old lib, though it was changed in branch…

@tobiasKaminsky
Copy link
Member Author

What shall I do? I merged it ~45min before your comment :-)

@AndyScherzinger
Copy link
Member

Nothing all. Good it seems.

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