On Fri, Apr 08, 2022 at 04:04:07PM +0200, Miroslav Zagorac wrote: > > - 0014-MAJOR-opentracing-reenable-usage-of-vars-to-transmit.patch > > => while I'm fine for 2.6, I'm really not for 2.5 without a big > > compelling reason. It's a feature addition, not a bug fix. > > Either there are currently no opentracing users in 2.5 due to > > this problem and this will serve nobody, or there are and there > > is a significant risk of breaking something for them, which would > > possibly encourage them to revert an update and keep other unrelated > > bugs. Thus is it absolutely mandatory that we backport this to 2.5 ? > > For example, is it currently working so bad that users complain and > > that it prevents them from upgrading from 2.4 to 2.5 ? That's the > > type of situation that can justify a backport of such a patch. > > In my opinion this is not a feature addition but a restore of functionality > that was temporarily disabled due to a change in working with variables > (i.e. saving a hash instead of a name).
Sure it was disabled, but *before* the release. This is the essential point here. Users running on 2.5 already do not have it. > This should not bring any drawback to the users as this functionality has > not worked so far in the 2.5 branch and if not used in the configuration it > does not change anything. My concern is essentially this one: is there any risk that a 2.5 user currently using opentracing would be hit by a bug introduced with this patch ? The code *seems* to be isolated only in the parts that are enabled by OT_USE_VARS but I'm asking because you know better than me. Don't get me wrong, you're the OT maintainer, and if as the maintainer you tell me "I need this to be backported", I'll obey. However my job as the project's maintainer is to warn you sufficiently about the fact that this completely violates the standard maintenance rules and that if done it must be done for a valid reason and not just because the patch applies well and probably it can make some users' life better. The general rule of thumb for backports is very simple: "will users yell louder at me for breaking their setup with a patch that wasn't needed than those who're currently missing that patch". If you have a solid reason, you can justify your choice and maintain it (or sometimes revert). If instead you just say "ok ok let's remove it, I just wanted to help", that's rarely perceived as a valid reason for backporting as that was something to be done during the development cycle (as we're doing right now with 2.6). > Maybe some user used this functionality in version 2.4 and it prevented him > from switching to version 2.5 because it doesn't work there? It's possible, but I'm not the best placed to gauge that ;-) > > Regarding the 2.4 series, I'm seeing 5 cleanup patches, but I haven't > > checked if they were needed or not. Normally we do not backport code > > cleanups in stable branches unless they are tiny, riskless and help to > > get other patches backported by providing the required patch context. > > One of the reasons is that some users have patches on top of these > > branches and applying cleanups that are not necessary sometimes causes > > them trouble to re-apply their local patches. It's unlikely that users > > have patched opentracing but that's a general rule, I think you get the > > idea. > > These cleanup patches are not necessary for functionality, so you don't have > to apply them. OK that's perfect. I'm trying a build and merging your set in 2.6 now. Thank you! Willy
