lgtm The comments were all about (potential) speed over obvious correctness. Your call. http://codereview.appspot.com/4532086/diff/1/apiclient/model.py File ...
14 years, 1 month ago
(2011-05-26 19:18:16 UTC)
#2
http://codereview.appspot.com/4532086/diff/1/apiclient/model.py File apiclient/model.py (right): http://codereview.appspot.com/4532086/diff/1/apiclient/model.py#newcode327 apiclient/model.py:327: if key not in modified: On 2011/05/26 19:18:16, aiuto ...
14 years, 1 month ago
(2011-05-26 19:42:07 UTC)
#3
http://codereview.appspot.com/4532086/diff/1/apiclient/model.py
File apiclient/model.py (right):
http://codereview.appspot.com/4532086/diff/1/apiclient/model.py#newcode327
apiclient/model.py:327: if key not in modified:
On 2011/05/26 19:18:16, aiuto wrote:
> Just a thought.
> It might be slightly faster as
> modified_value = modified.get(key)
> if not modified_value:
> ..
>
> and then modified[key] => modified_value in the other 2 instances.
>
> Also, maybe the outer loop as
> for key,value in original.iteritems():
Done.
http://codereview.appspot.com/4532086/diff/1/apiclient/model.py#newcode328
apiclient/model.py:328: patch[key] = None
On 2011/05/26 19:18:16, aiuto wrote:
> It took me a few minutes to realize that this was indeed a delete of the
> element. Could you add some comments to indicate the effect of the various
> replacements.
Done.
Issue 4532086: Add makepatch() method.
(Closed)
Created 14 years, 1 month ago by jcgregorio
Modified 14 years, 1 month ago
Reviewers: aiuto
Base URL:
Comments: 4