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

Kent Gibson warthog618 at gmail.com
Mon Dec 3 08:38:59 GMT 2007


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



Alexander Belchenko wrote:
> 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*.
>
Actually, as I read, "If no one has objection, I'll tweak the code in
question and merge it.", you volunteered to tweak my patch before
merging and are now sitting on it, effectively blocking it.

The code is functionally identical to what was already there, so I
don't see why it cannot be merged as is.  I haven't looked at the
wrapping issue you are concerned about and have no interest in doing
so.  If you want it fixed please fix it yourself as you indicated you
would.

>> 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.
Then you assume wrongly.
Wrt the import changes all I understand is that my patch is
functionally equivalent to what was there.  I don't need to understand
what the lines actually lines do in detail to understand that.
Separation of concerns.

> 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.
>
That's partly my fault, true.  I foolishly cleaned up some redundant
imports in my initial patch and Robert Collins wanted the imports
cleaned up properly.  Now you want this wrapping issue fixed cos that
touched a line you object to.  I guess I should've left well enough alone.

>> 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.
That is because you insist on having a line 100% correct before it can
be merged.
Why not apply that principle to functions, classes, files,...?

>
>> 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.
>
If you are ok with my patch then merge it, and if you want you can
tweak it on the way.  But if you aren't about to make that tweak NOW
then the how about merging my patch and fixing the other problem
separately?

> So, I don't understand your objection.
>
That much is certain.

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

iD8DBQFHU8CigoxTFTi1P8QRAl0WAKCuAf/l5lYmNo5xnqAL0nxpZKas/ACguy5C
6llSV7Azd7SPgpGdvYsRTRI=
=s0Re
-----END PGP SIGNATURE-----



More information about the bazaar mailing list