Re: [PATCH] Drop redundant 'const' keyword from object parameters

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

 



Hi,

On 17 November 2017 at 14:13, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> Fri, Nov 17, 2017 at 02:09:02 PM +0100, Christophe Fergeau wrote:
>> Fri, Nov 17 , 2017 at 11:21:21 PM +0000, Daniel P. Berrange wrote:
>>> On Fri, Nov 17, 2017 at 11:57:29 PM +0100, Christophe Donkerwolcke wrote:
>>>> Hi,
>>>>
>>>>>> It's definitely not used consistently, but removing a 'const' from
>>>>>> external API is going to cause breakage in C ++ code:
>>>>>
>>>>> Well that's true but IMHO this breakage is OK, given: >>>
>>
>> >> >>* the unlikelyhood of an existing C++ project using libosinfo.
>> > >
>> > > This patch would fix generation of rust bindings, which have 0 users
>> > > for sure at the moment since it's broken. So there are at least as many
>> > > C++ projects using libosinfo, maybe more :)
>> > > If the breakage is fixed in the Rust binding generator soon, I'd rather
>> > > we drop this patch.
>> >
>> > I'm on the fence, but I think on balance i'm slightly in favour of  taking
>> > in the patch. It doesn't change ABI, only API so it would only be source
>> > level incompatible. Even then its only incompatible if the dev actually
>> > declared the variable const, which is about as likely as someone using
>> > C++ in the first place.
>>
>> Yes, your point about the actual object most likely being declared
>> without the "const" modifier also makes me lean towards taking it.
>
> As usual, forgot to finish the email, but
>
> Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>

Thanks for reviews folks. Pushed.


-- 
Regards,

Zeeshan Ali

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux