A possible rule is to not make new nxstyle issues.
You can run nxstyle twice, before the patch and after the patch, and check
the difference to see if there are new violations.

It may not be accurate but basiclly works, and committers do not need to
check it manually to find out whether the patch introduces new violations.

Xiang Xiao <xiaoxiang781...@gmail.com> 于2020年3月11日周三 下午3:42写道:

> 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
>

Reply via email to