Add head property to FileDownloadDelegate's Progress/Response struct#811
Add head property to FileDownloadDelegate's Progress/Response struct#811Lukasa merged 10 commits intoswift-server:mainfrom
head property to FileDownloadDelegate's Progress/Response struct#811Conversation
|
summoning @Lukasa and @dnadoba since this was discussed previously here: #680 (comment) |
|
Hi @gregcotten, thanks for opening this and sorry for the slow time to review, it's been a busy week. I'll take a look now. |
Lukasa
left a comment
There was a problem hiding this comment.
Great, left a note in the diff. Can we also add at least one unit test?
it will never be seen by the library user with this data since `didReceiveHead(...)` is called before we report progress or finalize
|
OK @Lukasa I, at my own risk, have diverged from your suggestion as I think it's really important that the I've also added its use to existing unit tests, but can make a unique unit test if you have any suggestions. |
|
Needless to say this should be a squash merge if accepted :) |
Lukasa
left a comment
There was a problem hiding this comment.
Very minor nits here but this is looking good. I'll kick off CI for some more info there as well.
|
Looks like the formatter wants a few tweaks from you. |
Done! |
Lukasa
left a comment
There was a problem hiding this comment.
Awesome, I really like this.
I needed a way to use a
FileDownloadDelegatetask to fish out the recommended file name.The
headproperty is an explicitly unwrapped optional because there is no "default value" to set it to, and it won't be accessed by the user until it's already been set anyway. This is a little inelegant, so I could change it to something like below where I fill in bogus init data, but that seems worse for some reason.