zorba-coders team mailing list archive
-
zorba-coders team
-
Mailing list archive
-
Message #09926
Re: [Merge] lp:~zorba-coders/zorba/feature-fetch_binary into lp:zorba
> 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.
References