[PATCH stable] convert: convert real global tags only - not local tags

[Pierre-Yves David]

On 05/15/2014 05:32 PM, Mads Kiilerich wrote:
> On 05/15/2014 09:16 PM, Pierre-Yves David wrote:
>>
>>
>> On 05/15/2014 12:06 PM, Sean Farley wrote:
>>>
>>> Pierre-Yves David <pierre-yves.david at ens-lyon.org> writes:
>>>
>>>> On 05/14/2014 04:50 AM, Mads Kiilerich wrote:
>>>>> # HG changeset patch
>>>>> # User Mads Kiilerich <madski at unity3d.com>
>>>>> # Date 1400068180 -7200
>>>>> #      Wed May 14 13:49:40 2014 +0200
>>>>> # Node ID 6878d40f8a2153885374ba636a3b183a5a369d36
>>>>> # Parent  62a2749895e4151f766a4243fa870b1ddd7386d0
>>>>> convert: convert real global tags only - not local tags
>>>>
>>>>
>>>> Not sure to understand what this patch is about.
>
> This is about convert and tags, local and global.
>
>>>>
>>>> What is the problematic behavior before this patches.
>
> Convert did not only convert global tags - it also converted local tags.

But why is converting local tag bad? Sound perfectly reasonable things 
to do from here.


>> Sounds requires a description udpate -and- a comment in the code.
>
> It is ridiculous to have to describe the basic functionality of a sub
> system every time a patch touches it.

It is ridiculous to receives patches for review that introduces 
significant behavior changes without any justifications.

We cannot both encourage everyone to do some review and send them 
cryptic patches that requires paying attention to 3 week of prior 
discussion (on another channel?).

I agree we cannot explain every small details. But your patch should be 
processable by a decently wide audience. Including the guy that will use 
annotate in 6 months to figure out what is going on.

Also if you do not add a comment in the code about why we are converting 
global tags only. In 3 months we'll receive the opposite  changeset that 
enables conversion of local tags.


> What comment would you add? "we only consider the tags from taglist
> where the type is global"?

I do not want just a useless re-phrasing. You are suggesting an update 
that, to me, look like

   paint car in red

   Apply red paint on car.

I would like something that looks like

   paint car in red

   Car used to be black, a color reserved for secret service and bad
   guys usage. We re-paint it in red as red car are known to be faster
   anyway.

So please update this description and comment your code.

-- 
Pierre-Yves David