commit in brisbane core?

[Robert Collins]

On Thu, 2009-04-02 at 16:49 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > Could we remove this change please :) its a bogus condition to test for.
> > 
> >          if self.new_inventory is None:
> > +            # XXX: Using these asserts causes test failures. However, at least
> > +            #      "self._recording_deletes" seems like a useful check to do,
> > +            #      as it ensure the delta is completely valid. Most likely this
> > +            #      just exposes that the test suite isn't using CommitBuilder
> > +            #      100% correctly.
> > +            # if (not self.repository._format._commit_inv_deltas
> > +            #     or not self._recording_deletes):
> > +            #     raise AssertionError('new_inventory is None, but we did not'
> > +            #         ' set the flag that the repository format supports'
> > +            #         ' partial inventory generation.')
> > 
> > 
> 
> I'm fine removing it if you are sure it is okay.
> 
> However, if self._recording_deletes is not set, then the calling code
> hasn't guaranteed that we will properly have 'delete' called. I suppose
> for 'record_iter_changes()' it is new code, but it seems like it should
> conform to the existing requirement of informing about "recording_deletes".

recording_deletes is matched with record_entry, record_iter_changes is a
wholly different codepath. Uhm, either it should be activated and
tested, or deleted though. I don't have a strong opinion about doing it
either way.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090403/a91ca8ff/attachment-0001.pgp