-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Replaced double allocation by a single one. #2672
Conversation
Hey people, any thoughts about this? Seems quite simple. |
That was weird, is like Travis never even started. |
This is the Travis log:
@kcharwood I've seen you are taking good care of this project. This seems like a pretty much straightforward PR, could you please review it? I'm not sure who can decide whether to merge it or not. Thanks. |
Looks OK now. +1 |
@gfzabarino That travis log has been the death of me. It's just appears to be totally random and we have to kick it back off. I'll take a look at this one this week. I've been consumed with 2702, which I think we are almost out of the woods on. It's super pressing to get that one out the door, then I'll circle back on these. 🍻 |
Also, @gfzabarino do you think we need any additional unit tests? I would love to see tests with just about every PR we try to get in going forward. Just to make sure we prevent as many regressions as possible |
@kcharwood I guess I could write tests to make sure wherever final NSData is passed it is always the same instance. But if in the future someone writes a new way of delivering the downloaded NSData to the client, I have no way of testing for that, since it is still not coded :) Having said that, would you like me to still code some tests about the current implementation? |
I'd be interested in at least seeing your ideas on how it could be tested and verified. |
Hey @gfzabarino I apologize for just now getting to this. I'm ready to make it happen. I did a slight refactor here: 79a8d58 The two issues I wanted to address were removing setting two variables on the same line, and getting that |
Hi @kcharwood. One small comment. In the original implementation (check 29da5e5), if there's an error, the dictionary entry with |
Good catch @gfzabarino How about 1167d09? |
Yup that looks like 👍 to me @kcharwood. Thanks for wrapping this up. |
Merged in with 1167d09 Thanks! 🍻 |
These small changes saves precious memory. Particularly useful when dealing with large files.