Skip to content

Conversation

@AndyScherzinger
Copy link
Member

Folder size is now shown by remote computed folder size.

originally opened by @tobiasKaminsky on oC repo, rebased to latest master now.

According to the original discussion in the PR 1655 this will not work for Servers with oC version below 8 so it will always work for Nextcloud version > 9.x. So we are imho save here.

Please review @przybylski @tobiasKaminsky (I will do one too).

Since this is a minot change we might add this to the 1.2.0 release. Comments? :)

@AndyScherzinger
Copy link
Member Author

Code LGTM

* Sorts list by Size
*/
public static Vector<OCFile> sortBySize(Vector<OCFile> files){
final Integer val;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final int multiplier = mSortAscending ? 1 : -1;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed by 174fa1d

@przybylski
Copy link
Member

👍

@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Jul 13, 2016

Fixed @przybylski code review comments 👍
LGTM

@przybylski
Copy link
Member

przybylski commented Jul 13, 2016

👍

@AndyScherzinger AndyScherzinger added this to the Nextcloud App 1.2.0 milestone Jul 13, 2016
@AndyScherzinger
Copy link
Member Author

AndyScherzinger commented Jul 13, 2016

@tobiasKaminsky please check and merge at will ;) Originally it is your PR, so it is yours to push the button :)

@tobiasKaminsky
Copy link
Member

LGTM
this thing drives me crazy...

@tobiasKaminsky tobiasKaminsky merged commit 9062f37 into master Jul 13, 2016
@tobiasKaminsky tobiasKaminsky deleted the folderSize branch July 13, 2016 18:50
@tobiasKaminsky
Copy link
Member

Is this working for you? For me, I do not see any remote folder size :/
@AndyScherzinger @przybylski

@AndyScherzinger
Copy link
Member Author

Erm...just checked and I don't see any either :(

@AndyScherzinger
Copy link
Member Author

@tobiasKaminsky I fixed this here: #235 - please review and merge :)

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.

3 participants