On 12/9/13 1:56 PM, Ivan Zhakov wrote:
> I was worried a but that we have to care that mergeinfo --log will
> never have include_merged_revisions option.
I'm not sure there really is a problem with supporting include_merged_revisions
for mergeinfo --log. For instance even with our (Subversion project) branching
model there are cases where you might want to see the commit logs for the
development that happened on a feature branch. As things exist right now with
mergeinfo --log, no changes should be necessary on the log receiver to support it.
I realize it's not entirely straightforward as to when you'd want this and I
see no reason to rush into implementing it at this point.
> I didn't worry about allocation of empty apr_array_header_t of course.
>
> After further looking to r1548334 I would suggest use merge_stack as a
> flag for printing merged revisions or not, instead of deferred
> allocation. I mean if merge_stack is non-zero then handle children
> revisions, otherwise do not. But I'm also with always allocating
> merge_stack member, since code is more clear when we don't have to
> care to perform deferred allocation of apr_header_t.
I didn't intend to shift to deferred allocation when I started, but I did so
because it optimized the common case (no -g) and avoided locking the mergeinfo
--log implementation into not supporting -g without changes.
I don't think using the merge_stack baton member as a flag to ignore merged
revisions works very well. You either need the stack or a depth int to keep
track of how many SVN_INVALID_REVNUM's you need to see before you start
printing revisions again. This is true because children can be nested (e.g
r1234 on 1.8.x merged from commit to trunk in r1222, merged from commit to
feature_branch in r1000).
So if you want to be able to deal with receiver log data that has children
without displaying the children you'd need a new flag on the baton. Given that
I can't imagine a scenario where you'd ask for merged revisions and then choose
not to print it the data in the command line client, I don't really see a
reason to worry about it. Especially when if you wanted to provide this it'd
be a trivial thing to implement (new boolean in baton and some conditionals in
the log receiver code).
I agree that the code is more clear without deferred allocation, so if we want
to revert r1548334 I have no objections.
Received on 2013-12-10 00:15:51 CET