Having gone through a cycle of fixing a medium-sized PR recently, here are my thoughts:
- 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. - If the tool works, it's not very hard to fix a lot of style issues... it's boring work, but can be done quickly. - Making the tool only look at the differences seems complicated and not what we want in the long run. -adam On Mon, Mar 9, 2020 at 6:02 AM Gregory Nutt <spudan...@gmail.com> wrote: > > > Yes, that is why I send this email to get the community feedback. > > To avoid the topic extend to the unrelated field, let's just focus on > > one thing I suggest: > > The github action just check the coding style for the modified lines > > in the patch, not the whole source lines. > > I am in favor of keeping the full nxstyle check. We have talked about > this before. I agree it is painful now, but I think it is necessary. > If we do not do the full nxstyle check, then when will be do the full > nxstyle check? Never. So the coding standard problems will never be > fixed and the condition persists indefinitely. > > There are many files and quite a few of them have one or more coding > problems (and some have very many.. like lots of files under arch). But > if we are persistent in running nxstyle on the whole files, that > difficulty will gradually > > > 2.The github action still ensure the modified lines confirm the coding > > standard, so the relaxtion don't make the situation worse. > It does in a way, because the coding standard problems are never fixed > and will just return to bug us in the future. > > 3.The contributor or committer still can create the dedicated patch to > > fix the style issue. > > Please reply with your reason here if you have different opinion, so > > the community can get more thought. > > > >> I would not suggest having a vote on this in such a short time frame. > > I am not sure a vote is even required in this case. Xiang and Liu are > the owners of the CI system. My understanding is that they are simply > asking for input in order to best coordinate with the community. Thank > you for that. But I don't think they are bound by the opinions of other > and, at the bottom line, should do what they believe is best. > > > One thing that would be helpful would be to create a separate coding > style fix-up up effort. That could make the pain of full file checks > shorter in duration. We could run nxstyle on every .c and .h file. > > Many tiny fix-ups can easily be automated (such as, "Need a blank line > after _____" or "Need a space before _____"), without requiring a > massive complex (and ultimately impossible) pretty printer. 90% of the > nxstyle fix-ups could be handled this way. I think we could make the > job less painful if we did something like this. > > No automated tool can properly handle things like breaking long lines. > All tools that I am familiar with do very bad job of that, so some > fix-ups would, I think, still be manual. > > All re-formatting tools that I am familiar with degrade the coding > style, none fix it. > > > -- Adam Feuer <a...@starcat.io>
