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

Re: [PATCH 00/23] further population of xen/lib/



On 01.04.2021 16:55, Julien Grall wrote:
> 
> 
> On 01/04/2021 15:27, Jan Beulich wrote:
>> On 01.04.2021 16:04, Julien Grall wrote: >> So overall, the number of 
>> functions requiring overriding will likely be
>>> pretty limited and #ifdef would be IMHO tolerable.
>>>
>>> Although, I would be OK with creating a file per function that are
>>> already overrided. For all the others, I think this is just pointless.
>>
>> Well, I don't see a reason to special case individual functions.
>> Plus any reasonable static library should imo have one (global)
>> function per object file in the normal case; there may be very
>> few exceptions. Drawing an ad hoc boundary at what currently has
>> an override somewhere doesn't look very attractive to me. Plus
>> to be honest while I would find it unfair to ask to further
>> split things if I did just a partial conversion (i.e. invest yet
>> more time), I find it rather odd to be asked to undo some of the
>> splitting when I've already taken the extra time to make things
>> consistent.
> 
> I am sure each of us spent time working on a solution that some 
> reviewers were not necessary convinced of the usefulness and they had to 
> undo the series...
> 
> In this case, you sent a large series with close to 0 commit message + a 
> small cover letter. So I think it is fair for a reviewer to be 
> unconvinced and ask for more input.
> 
> You provided that now, I think we want a short summary (or a link to the 
> conversation) in each commit message.
> 
> This will be helpful to understand why the move was made without having 
> to spend ages finding the original discussion.

I'll add "Allow the function to be individually linkable, discardable,
and overridable." to all the str*() and mem*() patch descriptions. Is
that going to be good enough?

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.