Code review comment for lp:~abentley/launchpad/bulk-insert

Revision history for this message
j.c.sackett (jcsackett) wrote :

Aaron--

This looks good; I agree with Rick's request for a bit more comments in newFromBazaarRevisions--while it eventually becomes clear what data a given loop or clause is assembling, I wouldn't mind a quick "# Get parent data out of revisions" or similar to help a reader keep track of what's going on where.

Rick--

You're right that the *args format would be cleaner in some cases, but here it looks like the more common case is that the param passed in is a list already, in which case the function would have to do something like `revisions = args[0]`. While I agree it may be a good idea to start raising the use of the *args pattern more, I don't think it's a net gain here.

review: Approve

« Back to merge proposal