On Tue, Mar 10, 2020 at 3:43 PM Xiang Xiao <xiaoxiang781...@gmail.com> wrote: > > On Tue, Mar 10, 2020 at 1:17 AM Gregory Nutt <spudan...@gmail.com> wrote: > > > > > > > - Fixing the entire file we touch helps makes things better overall > > > even > > > though its painful in the short run. > > > - Having a tool that produces accurate style recommendations is very > > > important— it's hard to know what to fix and what not to fix if the > > > tool > > > doesn't give accurate messages. > > > > I don't know how long it will take, but these two things converging is > > the keep to ending the pain permanently: Fixing non-compliant files and > > improving the tools. > > > > I think we should at least wait a little while. If it takes too long > > and does not converge to a reasonable solution in the near future, we > > can revisit this. > > Ok, let's wait for sometime to see the result. Since the workflow > isn't lockdown yet, but the precheck is running for each PR now, I > would suggest that PR must pass the precheck before merging to ensure > the basic quality.
do you have any suggestions what to do with this PR? https://github.com/apache/incubator-nuttx/pull/533 all nxstyle errors are known and explained in the commit message. > > > > > > - Making the tool only look at the differences seems complicated and > > > not > > > what we want in the long run. > > > > nxstyle does already support this option: > > > > $ tools/nxstyle.exe -h > > nxstyle version 0.01 > > > > Usage: nxstyle [-m <excess>] [-v <level>] [-r <start,count>] <filename> > > nxstyle -h this help > > nxstyle -v <level> where level is > > 0 - no output > > 1 - PASS/FAIL > > 2 - output each line (default) > > > > The -r option specifies a range of line numbers to test. > > > > So it is not complicated for nxstyle (CI logic would need some > > extension, however). The real question is whether we should do that > > rather than if it is difficult to do. > > > > CI logic is also ready now, we just need pass -r to checkpatch.sh > which will generate the list of start/count for nxstyle to just cover > the modified lines. > > > > >
