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
