nits about get_revisions...

[Robert Collins]

On Thu, 2006-07-06 at 12:43 +1000, Martin Pool wrote:
> On  5 Jul 2006, John Arbash Meinel <john at arbash-meinel.com> wrote:
> > Robert Collins wrote:
> > > Just a note, I think this stuff should have been caught by peer review:
> > > 
> > >                                                     self.get_transaction())[0]
> > >     @needs_read_lock
> > >     def get_revisions(self, revision_ids):
> > >         return self._revision_store.get_revisions(revision_ids,
> > >                                                   self.get_transaction())
> > > 
> > > 
> > > Hopefully that pasted correctly. Anyway:
> > >  - no docstring
> > >  - no vertical space between the previous function and this.
> 
> OK, docstrings definitely should be reviewed.
> 
> On the other - I'm all in favour of consistently formatted code, but
> having zero or one or two lines of whitespace in particular places is
> fairly far down my priority list, to the extent that I'd rather just fix
> it later than make people iterate their submission.

I'm fairly sure you are stating that for clarity, so I'd like to note,
also for clarity, that I didn't put any assessment of priority about the
two items.

When I say 'caught by review', I dont mean 'force another iteration' - I
mean 'should be noted by someone as something to do'.

With all our reviews, there is always a judgement call as to when a
given aspect should be improved:
 - some point in the future
 - before merging, but dont bother posting to the list again
 - before merging, and lets see the code again.

IMO the point of the review process is to identify as many things we can
improve as possible, and put them into one of those three categories.

-Rob


-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060706/a3e71439/attachment.pgp