On Thu, Nov 14, 2013 at 5:12 PM, Jeff Law <l...@redhat.com> wrote:
> On 11/14/13 13:28, Diego Novillo wrote:
>>
>> Functions in each corresponding .c file got moved to those
>> headers and others that already existed. I wanted to make this
>> patch as mechanical as possible, so I made no attempt to fix
>> problems like having build_addr defined in tree-inline.c. I left
>> that for later.
>
> This seems backwards to me and just ensures double-churn. Once to move it
> now, then again to its final resting spot.
>
> If this change is being made via some automated script, then, well, I guess
> it is what it is and we'll have to come back to them.  But if you're doing
> this by hand it seems to me that leaving it in its original location,
> possibly grouped with its friends, with a FIXME would be better.

Most of it was automated.  I want to stage it in, and I worked pretty
hard at not making additional changes. Particularly since it is not
clear where we will want some of these functions to end up in.  So, we
will still need several passes.  Making each pass self contained makes
sense to me.


>> - Some header files always need another header file. I chose to
>>    #include that header in the file. At this stage we want to do
>>    the opposite, but this would've added even more bulk to the
>>    change, so I left a FIXME marker for the next pass.
>
> This seems a bit like a mistake.  How much of this patch would be blocked if
> we didn't allow this right now.

A good chunk.  I'm doing these FIXMEs in the next sequence of patches,
so we won't have them for long. Again, I was going for an orderly
transition here.

Diego.

Reply via email to