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
