On Wed, Mar 11, 2020 at 8:13 AM David Sidrane <david.sidr...@nscdg.com> wrote:
> First, I have to say that I value the NuttX coding standard - it make for > the most readable code I have EVERY worked on. I want it preserved. > > But in light of the below discussion: I am changing my position that the > "rule" CI is to just check ONLY the changes to a file AND if the only CI > failure is a minor style failure the committer may commit it. > > To be clear I am not saying that a foreign camel cased contribution is > allowed in. Just small stuff. > > Rational: > > Way back, on the list, and in email it was decided that Job #1 was having a > valid style check tool. > For some it was a foregone conclusion that if CI did style checking the > tool > had to 100% accurate and provide go no go results. > > If we allocated more time fixing the tool, than reinventing ways to run CI, > we would not be in the state we are in. The message here is: We should do a > better job prioritizing and avoiding reinventing the wheel. > > Why avoid the root cause. We need to fix the tool. > > Now from a what is required to move forward point of view: We have been > living under a belief that all the code in the OS 100% conformed to the > coding standard, and expected that the violations were mostly in arch. None > of this has proven true. The reality of what this means is, that we will > have to touch every file in the system, (this was expected, but just for > licenses changes). Running a constantly changing, and sometimes broken tool > constantly on every file in the system is a waste of resources. So we need > to fix the tool _THEN_ fix the code with it. I suggest, until that happens, to do two things in parallel: (1) make it the interim policy that *changed* lines have to conform. Over time this will reduce nonconforming code. (2) during the process of fixing licenses, fix also the coding style of affected files. In other words, if changing code, changed lines must conform; if changing license, make whole file conform. Rationale: Commits that make functional change should be limited to just that functional change and nothing else, for purposes of review, revert, etc. (License change is not functional change.) Cheers Nathan
