[MERGE] Support logging single merge revisions with short and line log formatters.

Alexander Belchenko bialix at ukr.net
Sun Dec 2 14:47:35 GMT 2007


Kent Gibson пишет:
> Alexander Belchenko wrote:
>> Alexander Belchenko has voted tweak.
>> Status is now: Conditionally approved
>> Comment:
>> I want to note that following code is incorrect. It was incorrect
>> before and still incorrect now:
>>
>>     if to_file is None:
>>         to_file = codecs.getwriter(user_encoding)(sys.stdout,
>>                                                   errors='replace')
>>
>> You should not use user_encoding unconditionally.
>> If bzrlib want to create wrapper around stdout it should use
>> osutils.get_terminal_encoding().
>>
>> This patch is not about terminal encoding. But Kent [accidentally]
>> touched this code. So I think this piece of code should be fixed as
>> well now. This piece of code does not have proper test before, so
>> I'm not intended to write new one now.
>>
>> If no one has objection, I'll tweak the code in question and merge it.
>>
> Well I object.  To block my patch because it makes an incidental
> change to a line which is broken for other reasons makes little sense
> to me.

I don't understand you. I approved your patch and give clear explanation 
that in current form it's incorrect in one place, so it's need 
*tweaking* *before* *merging*.

> As you pointed out yourself, my change is not relevant to the problem
> you want to fix.

It's your patch. I don't want to fix anything. You're touching codes --
I assume you should understand what you're doing. Even in the case if
changes are mechanical. Your initial patch was not about fixing all 
imports in log.py, but your second one is. So if you or someone else
start to clean-up old garbage in the code the work should be completed.
IMO.

> You are linking my patch to a totally independent problem, and the two
> should be kept separate.

Your patch is a single whole change for me.

> So my suggestion is to review my patch in isolation, then go ahead and
> fix the other problem.

I reviewed your actual code and it was OK for me. But the way used to 
creating wrapper around stdout is not OK for me. It's not blocker for 
merge. I just want tweak here. I even able to do it myself, when I'll do 
  prepare it to PQM. That's all.

So, I don't understand your objection.




More information about the bazaar mailing list