zorba-coders team mailing list archive

[Matthias Brantner <matthias.brantner@xxxxxxxxxx>]

> Looks good - even the transcoding stuff seems to work :)
> 
> Some remarks (nothing blocking):
> - It would be nice to keep fetch-binary functions together in the module.
fixed

> - Is internal::StreamResource::create ever used with 3 parameters?
for http stuff when coming from the api

> - fetch_impl.cpp:
>   - 104
>     Why is lEncodingStr initialized if it is always overwritten in line 115?
>     Also, as the lEncodingStr is only accessed using c_str(), the zstring is
>     probably not necessary at all.
fixed

>   - 115
>     It seems that dynamic_cast<internal::StreamResource*> and the error
> handling
>     could be moved into getFetchResource.
fixed

> - It would be great to have at least one http-Test (but of course that adds
>   flakiness to the test run …)
not fixed for this reason
-- 
https://code.launchpad.net/~zorba-coders/zorba/feature-fetch_binary/+merge/105497
Your team Zorba Coders is subscribed to branch lp:zorba.