davids5 commented on issue #2: Master pr nxstyle improvments URL: https://github.com/apache/incubator-nuttx/pull/2#issuecomment-568456453 > @btashton @davids5 What I mean here is that we don't need modify Makefile, is engouh to add a script invoke nxstyle(maybe build it too) to check the style for specific file/files or commit/commits. > Then we instruct people integrate this script into pre-commit hook to enable the check automatically. Like: > https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl > Nobody try to integrate checkpatch.pl into Linux build system, since developer can run checkpatch.pl directly to: > 1.The bunch of specified files > 2.The bunch of specified commits > 3.The current living change > 4.Integrate to pre-commit hook > nxstyle(maybe build it too) That is make's job. > It's much flexible than make check_format since we can pass the different argument to script form command line, but the same thing is hard to do for Makefile. I agree - but adding it to the make file has some benefits and it does not prevent any entity (user, ci, tools) from running the script directly. There are a few subtle points here from a non-power users and system POV. We want to grow the community it is make for easy of use fot N00Bs 1. Most editors and ide's have make integration. 2. First normal form and abstraction. If we add the formatting targets to the `MakeFile` it is top down and use-able by the users, CI and other tools. `make check_format` `make check _format_all` and for the user (When, we have a decent tool) `make format` - that fixes the issues As of today `nxstyle` lives in `tools` and in tree. It may need to be moved. Think about the best practices with git when the release branch's nxstyle != master's nxstyle and master is correct nxstye (or a conf file for a real tool). It will level room for a whole class of errors we can avoid with a simple Workflow. Also consider the change to tools like when nxstyle++ is written or we use clang-format. Do you want to update the location and process everywhere: in the CI repos, containers and ./tools or do you want to update the makefile? @btashton , @xiaoxiang781216 Please set me straight if I am off course or too myopic. The things we get wrong now will become technical-debt. Granted at this point it in time it is small but when there are 300 committers on nuttx it will be a problem 300 times lager.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
