I don’t mind clean up patches, but this is the reason a lot of Linux kernel maintainers frown on them.

https://lore.kernel.org/all/2023120938-unclamped-fleshy-688e@gregkh/

This failure is because of a clean up patch that converted everything to “bool” where it could be:

https://lore.kernel.org/all/20230305155532.5549-3-ubizjak@gmail.com/T/#u

If I had not accepted that clean up, this backport would have been pulled in automatically with no extra work from myself. But because I added that clean up, I now have to fix this for every stable release before that clean up 🙁

@rostedt I am not a 'purest' in general and tend to dislike such changes for the sake of theoretical better but practical massive NOP that more often than not leads to more work with no or little return. There are exceptions though! Why false is better than 0 anyway?

@qyousef false better than 0 is more for understanding that the value is a boolean an not to be taken as numeric. Sometimes that makes it easier to understand the logic. I’ve been trying to use boolean in those cases as well. This also is a requirement if you ever plan on using Rust 😉

@qyousef Although, I just hit another patch that did not backport properly because of that change 😛 This means I’m going to NACK all clean up changes from now on, citing that it breaks backports.

@rostedt Fair enough. I tend to write newer code this way. But I don't see a need to change older code if the functions aren't being changed for another good reason 😅

@rostedt @qyousef I think it's suboptimal to prioritize easier stable backports over a better mainline.

@vbabka @qyousef I just wasted over an hour fixing patches that should have gone back automatically without my involvement. I don’t think 1/0 vs true/false is worth it. Note, I’m doing this on my own time, where I have other things I rather be doing.

@vbabka @rostedt Not all patches are equal of course. But I think some like this one fall into the noise. If they were lumped as part of other more meaningful changes in the same area it'd make more sense IMHO

@qyousef @vbabka Exactly. If you do clean ups in the code that you are modifying then all is OK, because the modifications you are making will cause the backports to fail anyway, so the clean ups do not cause extra work. But if you just have random clean ups in code that hasn’t changed in years, if a bug in that code is found, then the backports are going to be a pain fixing all previous version before the “cleanup”.

@qyousef @vbabka Which, BTW is exactly what happened here! 😛

@rostedt @qyousef what if the cleanup was backported too instead of adjusting the fix? IIRC @gregkh has no problems with that

@vbabka @rostedt @qyousef exactly, I'll gladly take dependent patches like that, would prefer to in fact, makes it simpler for everyone involved.

@gregkh @vbabka @qyousef Well the problem is that it still requires manual effort to even include the clean up patch. The point I was making is that if a clean up patch causes a backport to fail, I still have to look at why instead of it just nicely being pulled in by the stable tag. The clean up in question, touched much more than the areas that failed, so it too may not backport properly.