Review: Approve

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.
- Is internal::StreamResource::create ever used with 3 parameters?
- 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.
  - 115
    It seems that dynamic_cast<internal::StreamResource*> and the error 
    could be moved into getFetchResource.
- It would be great to have at least one http-Test (but of course that adds 
  flakiness to the test run …)

Your team Zorba Coders is subscribed to branch lp:zorba.

Mailing list:
Post to     :
Unsubscribe :
More help   :

Reply via email to