On Fri, Apr 7, 2023 at 1:13 PM Tommy Murphy <tommy_mur...@hotmail.com> wrote: > > Hi there > > As many of you probably know, RISC-V OpenOCD development continues to be done > on this fork: > > https://github.com/riscv/riscv-openocd > > Periodically, changes here are upstreamed to the "main" OpenOCD project > and/or patches upstream are pulled down to more closely sync/align the two > repos. > > However an issue that has arisen from time to time is that the latest > checkpatch: > > https://github.com/openocd-org/openocd/blob/master/tools/scripts/checkpatch.pl > > has been modified/updated such that previously commited code/patches now fail > the latest checks.
Hi Tommy, indeed cherry-picking old patches can result in checkpatch screaming hysterically ! I didn't have this case in mind when I added to checkpatch the possibility of dropping some failing tests. Checkpatch comes from the Linux kernel, where the patch review is done on the mailing list; it's known that checkpatch has several limitations, and maintainers accept patches that fail at checkpatch if the developer provides good reasons. Instead OpenOCD review is done in gerrit, and a failure with checkpatch can cause maintainers to ignore the patch. Dropping some check is sometimes mandatory, e.g. https://review.openocd.org/7568 https://review.openocd.org/7579 Adding 'Checkpatch-ignore:' in the commit message does the trick. It is explained inside HACKING Back to re-sync upstream and RISC-V fork, I don't know what is the easier or better way to proceed. Can you use Checkpatch-ignore in the fork? 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
