Code review comment for lp:~james-w/lazr.restfulclient/improve-collection-fetch-optimisation

Revision history for this message
Graham Binns (gmb) wrote :

Hi James,

Thanks for this branch. A couple of things:

 1. I don't see any tests for this change. Are there tests that already cover this functionality? If so, how will this change affect them? If not, can you add some?
 2. Formatting multi-line conditionals is always a bit weird. We usually wrap after the first 'and' or 'or'. I've written a patch that fixes this for this branch: http://pastebin.ubuntu.com/558064/

> Actually this doesn't allow you to limit the page size
> all the time. Once you get beyond the first page it
> makes a request with no size specified.
>
> In order to do this properly we would either need a
> max-size request variable, would that be feasible for
> LP?
>
 3. I don't really understand what you mean here (I'm still jetlagged, which doesn't help). Can you explain it further (possibly using words of no more than one syllable)?

review: Needs Information (code)

« Back to merge proposal