Code review comment for lp:~amanica/bzr/325618_log_returns_too_much

Ian Clatworthy wrote on 2009-10-20
> Sorry for taking months and months to get to this review. It's been a crazy time for me. I'll try to find some time now to work with you towards a solution we're both happy with.

Thanks a lot, without feedback I couldn't really continue. I've also been a bit occupied (holiday/work/studies).

> To be honest, I use qlog almost exclusively these days so I'm less passionate than what I was about how log behaves on the command line. I do agree that #325618 is a real bug, though I'm not sure I'm comfortable changing default behaviour to fix it. I certainly think "bzr log -n0 -rX" ought to show the merged revisions for X, so if this patch alters that, I'd vote against it.

ehm, I mis-analysed the problem and fixed it the wrong way the first time around.
In stead of chopping off all parents after the stop-revision, I now allow
the "merged revisions" of the stop-revision. I think that clears up all the
controversy and all the tests I mangled before could be restored to their
previous glory.
BTW. your phrasing here in the feedback helped the penny to drop. thanks again.

> On a semi-related topic, if we are going to allow/encourage alternative behaviour wrt revision range semantics in log, I like the idea of adding an option called something like --exclude-lower as Eric suggested.

Great, I added a draft spec now, to keep track of all this:
http://bazaar-vcs.org/DraftSpecs/NewLogBehaviour

> In your opinion, is it possible to fix this bug without changing semantics?
Yes. this merge-proposal should be self-sufficient now. i.e. I don't think
it needs any of the other proposed changes to be useful and bearable.

For the last time thanks for the feedback, it inspired me to start completing this stuff, finally.