search

tus / TUSKit

The tus client for iOS.
https://tus.io/
MIT License
209 stars 114 forks source link

Add an implementation to fetch previous uploads #156

Closed kiyosoft closed 1 year ago

kiyosoft commented 1 year ago

add a feature to get the previous uploads #155

Acconut commented 1 year ago

Looks good for my eye without Swift expertise :) @donnywals, can you review it?

donnywals commented 1 year ago

Look mostly fine to me, I'm not entirely sold on the naming though. Since this method leverages loadAllMetadata() internally we could call this at any time which means that it can include currently active uploads too.

I'm wondering whether it would make more sense to name the method getUploadInfo and to name the PreviousUpload object UploadInfo to make it a little bit more general purpose.

Also considering whether we should make the metadata object public but given that it's such an important part of lots of our internals it's probably a good idea to provide an immutable copy as its own type instead.

kiyosoft commented 1 year ago

I name it findPreviousUploads to make it consistent with the tus-js-client but giving it more generic name makes more sense. As you said lets change the PreviousUpload to UploadInfo, but for the getUploadInfo it is more like a single value return function but we expect a list of UploadInfos. @donnywals can you suggest another name for this?

donnywals commented 1 year ago

Fair point on the info vs infos. Maybe we could do something like getPendingUploads(), getStoredUploads() or something like that. I think on the method name the Info part can be implied

kiyosoft commented 1 year ago

getStoredUploads() looks good to me. I will change and update the PR shortly

EDIT: Updated

kiyosoft commented 1 year ago

@donnywals When do you think this will be merged? I need it for my implementation

donnywals commented 1 year ago

I will merge this today and prepare a new release on Monday. You can go ahead and point to main as a dependency until then