[MERGE] [BUG #4663] Fix ``bzr log -r`` to support selecting merge revisions.

[Kent Gibson]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



Andrew Bennetts wrote:
> Andrew Bennetts has voted +1.
> Status is now: Approved
> Comment:
>> +        if isinstance(start_revision,RevisionInfo):
>
> PEP 8 says there ought to be a space after the comma.
>
jam has mentioned that as well, but I still haven't found the relevant
section in PEP8.
(Sorry - waving PEP8 at me is like a red rag to a bull.)
The only section that gets close refers to avoiding extraneous
whitespace, not enforcing its presence.  While all the example code in
PEP8 does include the space you refer to, I still don't believe it
says it is a must.

But rest assured that my future patches will include said space, since
that seems to be the accepted bzr style.

> It's a shame that so much of log is tested via cumbersome blackbox
> tests, but that's not your fault.
>
I had a bit of a go at fixing that when I added the unit tests for my
verbose merge revision logs (already merged) - at least separating the
tests for the log framework from the tests for the formatters, and
moving some of the tests from blackbox back into the core
functionality tests.
It is still pretty ugly though.

> It would be good to add a comment about the filter stack (especially
> the ordering), just to make the way the code works a bit clearer.
True enough, though I generally prefer tests to comments.  I suspect
if you change the order the unit tests will break.  But if so it is
due to luck more than good planning.
So probably should add an explicit unit test to check stacked filters.

>
> I'm very much looking forward to this bug being fixed, and this does
> look an improvement all around, so +1 from me.

Thanks for the review.  Hope you find it useful.

> You have minor conflicts with current bzr.dev; I'll resolve them and
> send the merge request.
Well there wasn't two months ago when I wrote it :-).  I wouldn't be
at all surprised if there are conflicts with my unit test changes for
the verbose merge revisions.

Cheers,
Kent.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGi1kUgoxTFTi1P8QRAm3YAJ417wQMF/An9ZWFIMi09bl4N2Bd5wCgw8I9
PGTy2FxZ8llRZFXNaLZjo28=
=oMLp
-----END PGP SIGNATURE-----