Conversation
|
LGTM |
|
@LukeOwncloud @AndyScherzinger do you want to do a code review? |
|
Looks good to me 👍 |
| file.setCreationTimestamp(remote.getCreationTimestamp()); | ||
| file.setFileLength(remote.getLength()); | ||
| if (remote.getMimeType().equalsIgnoreCase("DIR")){ | ||
| file.setFileLength(remote.getSize()); |
There was a problem hiding this comment.
Ah, I almost missed this part, that's why I thought it couldn't be working.
@PVince81 , do you recall what's the minimum server version including the property oc:size ?
There was a problem hiding this comment.
Not sure, could be OC 6 or OC 7. That property has been there forever... But it's only on folders as far as I remember, because files have "getcontentlength" but folders don't have that one officially.
There was a problem hiding this comment.
Yes, you are right, that's what we need, the folders. Thanks!
There was a problem hiding this comment.
I am always trying to hide the important things ;) ;)
There was a problem hiding this comment.
LOL
Was checking the library, we explicitly request the oc:size property from owncloud/android-library#55 . We should make some minimum test with server 8.0.
Also would be wise to check what minimum OC server we grant the app works with. Do you recall it, @rperezb ?
There was a problem hiding this comment.
@davivel that's a good question!
For sure, I recall that we don't support oc4, I remember that the entrypoint changed that time (LOL), having said that, I will go with what we support as platform, we are about to stop supporting oc7 https://github.com/owncloud/core/wiki/Maintenance-and-Release-Schedule
@MTRichards is the one
|
With OC server 8.0.12 works fine. With OC server 7.0.14 all folders show 0B as size. Server 7 is ending its life cycle, but we should support it. Ideas to handle it:
|
|
I think option 3 is ok as the user still can sort all files by size. |
|
Having in mind that oc7 is not being supported any more, I agree with @tobiasKaminsky |
If that's the case, maybe we don't need the special cases. I'm not sure about server 8.0.0, but 8.0.12 is working correctly. Is it worth that we add special handling for possible differences between minor versions? |
|
Yes, checking https://github.com/owncloud/core/wiki/Maintenance-and-Release-Schedule seems that 7.0.15 dies this current month |
|
@davivel 3 days... |
|
If OC7 is not supported any longer then we can omit every check for the version, which makes the code less complicated 👍 |
|
Recovering focus on this. Works with all the maintained versions of server, so 👍 from code perspective. Unfortunately, there's too much already for 2.1.0, so needs to go to 2.2.0. @jesmrec , ping us before testing to rebase. |
|
@davivel, i'm going to start to work with this PR. If you can rebase it... |
|
Sure, in a minute. |
|
@jesmrec , rebased. |
davivel
force-pushed
the
folderSize
branch
2 times, most recently
from
ffa1e92 to
f643460
Compare
|
All is ready -> in |
7 participants
Folder size is now shown by remote computed folder size.