On Tue, 2021-10-19 at 05:26 -0700, Joe Perches wrote: > What I suggested is not a patch it's just an example. Sure, I understand. In this case, I'll take some inspiration from your example and break down the changes into smaller chunks, thank you. > There's quite a lot of code in that driver that _could_ > be updated/refined/refactored (none of which _I_ will submit), > but it's up to you do whatever _you_ want. Indeed there is, I'm trying to tackle one thing at a time. I thought I could fix a couple of line length warnings in an easy way but I was wrong. Ok, I'll come back to CamelCase squashing and removing the Hungarian notation before working on this refactor. I think this is a good candidate for a patchset. On Tue, 2021-10-19 at 15:07 +0200, Fabio M. De Francesco wrote: > Hi Karolina, Hi Fabio, Thank you for describing everything in such detail, really appreciate it. > No, there is no problem in using a[i - 1]. Personally I prefer the > former when 1 <= index <= ARRAY_SIZE(a). I see, thanks for explaining this. > If you code "index = index -1;" or > "index--;" (that is the same) and then you use 'index' many lines > below that decrement in "a[index]" it may be not immediately clear > that you are not indexing past the end of the array. That's what I thought as well. > I prefer to state it again: if you choose to do such kind of works, > be careful to split self-contained patches in a series and explain > each change you make and why you make it. > Each patch must do only one logical change. Will definitely do so, thank you. > Each patch of a series must be self-contained also in the sense that > it must build without introducing errors or warnings at any point: > for instance, five patches => five clean builds. Makes sense, will keep that in mind. Also, I think it would be good to mention it on the FPT page. I can suggest adding such comment in later on. Thanks, Karolina
