Re: [PATCH 07/14] secret: Move virSecretObjListGetUUIDs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/25/2017 09:56 AM, Pavel Hrdina wrote:
> On Tue, Apr 25, 2017 at 09:46:50AM -0400, John Ferlan wrote:
>>
>>
>> On 04/25/2017 08:42 AM, Pavel Hrdina wrote:
>>> On Tue, Apr 25, 2017 at 08:00:26AM -0400, John Ferlan wrote:
>>>>
>>>>
>>>> On 04/25/2017 04:39 AM, Pavel Hrdina wrote:
>>>>> On Mon, Apr 24, 2017 at 02:00:16PM -0400, John Ferlan wrote:
>>>>>> Move code closer to usage.
>>>>>
>>>>> NACK, the function is exported so the usage is not only inside this file.
>>>>> In addition it adds an extra step while tracing the history of the code.
>>>>>
>>>>> Pavel
>>>>>
>>>>
>>>> While I don't agree, but I can drop this - again, in patches not yet
>>>> posted it's a lot easier to read the code with the GetUUIDs closer to
>>>> what GetHelper (or what it becomes by patch9).
>>>
>>> I thing that it's OK to have both *Callback functions next to each other
>>> followed by the virSecretObjListNumOfSecrets and virSecretObjListGetUUIDs
>>> next to each other.
>>>
>>
>> Exactly, but you NACK'd the code motion.  Now I'm confused.
> 
> What I meant is that in this case the order of functions doesn't matter
> so the code movement is pointless, the only benefit is that it may look
> better if you scrolling throughout the code.  I don't want create a
> precedent that moving function just for cosmetic reasons is a valid
> reason :).
> 

That precedent ship sailed long ago.  Moving code for cosmetic reasons
and reordering code has been done numerous times. Many times to make it
easier to see differences in subsequent patches.

I can drop this patch, but once I get to patch 9 I'd keep the Callback
helper next to the external API. So it's either pointless now or then.

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux