On Fri, Apr 7, 2023 at 7:46 AM Antonio Borneo <borneo.anto...@gmail.com> wrote:
> indeed cherry-picking old patches can result in checkpatch screaming > hysterically ! > Note this is not the result of cherry-picking. It's simply merging all of mainline openocd up to a certain change into riscv-openocd. > Can you use Checkpatch-ignore in the fork? > Possibly. Part of the problem is that I haven't figured out in the github action how to find which changes are part of the pull request. Erhan suggested some clever git commands in https://github.com/riscv/riscv-openocd/pull/816#issuecomment-1478245399, but they don't work when the action is running. I guess we'll have to figure that out, and then we can add checkpatch-ignore, or simply override the check when merging. Tim > > Well, > https://github.com/riscv/riscv-openocd/pull/816#issuecomment-1474425966 > is clearly a corner case. I pushed an incorrect SPDX tag to comply > with the older checkpatch, then pushed the fix once the new checkpatch > was merged. > Maybe the easier way is to manually skip the -1 in github. > > For review.openocd.org, I would prefer to receive mainly patches that > pass checkpatch, without excessive use of Checkpatch-ignore. > > Feel free to report here any issue with checkpatch; hopefully we can > improve it. > > Antonio > > > This obviously causes problems with the merging of upstream patches > intoto the RISC-V fork. For example: > > > > https://github.com/riscv/riscv-openocd/pull/816#issuecomment-1474425966 > > > > I was just wondering if there were any opinions on the merit or > otherwise of going back through historical patches (and other already > committed code?) to ensure that they comply with the latest checkpatch at > any point in time? > > > > The main pros seem to be that it would make the OpenOCD source code more > compliant with checkpatch checks and make the pulling of OpenOCD patches to > a downstream fork more straightforward (the latter, perhaps understandably, > not really being a priority for the "main" OpenOCD project?). > > > > But I can certainly also think of some cons :-) - e.g. some patches are > allowed deviations from the checkpatch rules, messing with existing > committed code/patches risks destabilising things, why do RISC-V > development in a fork and not in the upstream project in the first place > etc. > > > > Anyway - I was just interested in feedback on this and, for what it's > worth, if going back through historical patches and getting them to pass > the latest checkpatch checks is considered "a good thing" I am willing to > help out if necessary. > > > > Thanks a lot > > > > Regards > > Tommy > >