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. David -----Original Message----- From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com] Sent: Wednesday, March 11, 2020 12:42 AM To: dev@nuttx.apache.org Subject: Re: Should we relax precheck a little bit? On Wed, Mar 11, 2020 at 2:49 PM Takashi Yamamoto <yamam...@midokura.com.invalid> wrote: > > we basically prevent changes on files unless you can fix all nxstyle > issues in them. > i don't think it's a good idea. > > i believe that there can be changes more important than style fixes. > in addition to -r option you suggested, > i'd suggest to make precheck separate from other (IMO more important) > checks like build tests > so that reviewers can choose to "ignore" nxstyle errors after inspecting > them. > Yes, it's possible another solution. I am fine with any approach: 1.Must fix nxstyle issue for all whole files 2.Must fix nxstyle issue for all modified lines 3.Make nxstyle optional, let commiter make the decision. The community could discuss with one is best. But once the rule is lockdown, we should try our best to follow the rule except some special case. My bottom line is that the PR must pass all build test before merging. > On Sun, Mar 8, 2020 at 3:11 AM Xiang Xiao <xiaoxiang781...@gmail.com> > wrote: > > > > Hi all, > > The precheck ensure the whole file content comform to the coding > > style, this strategy has several problems: > > 1.Many source file in mainline already violate the coding style > > 2.nxstyle frequently generate the false alarm in the current stage > > How about we let precheck just ensure the modified line don't violate > > the coding standards? > > I am asking this question because: > > 1.The change in PR 471 has a very good shape: > > https://github.com/apache/incubator-nuttx/pull/471/files > > but the whole file precheck complain there are many errors: > > > > https://github.com/apache/incubator-nuttx/pull/471/checks?check_run_id=492244725 > > It is unfair to require the contributor to fix the issue not made by > > them. > > 2.Most PR will fail at precheck stage due to item 1 and then block the > > more important build test. > > > > Thanks > > Xiang
