Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

David,

Here' the draft PR, marked [Do Not Merge]:

https://github.com/apache/incubator-nuttx/pull/583

-adam

On Tue, Mar 17, 2020 at 12:40 PM David Sidrane 
wrote:

> Hi Adam,
>
>
>
> See inline…
>
>
>
> *From:* Adam Feuer [mailto:a...@starcat.io]
> *Sent:* Tuesday, March 17, 2020 12:10 PM
> *To:* David Sidrane
> *Cc:* dev@nuttx.apache.org
> *Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
> precheck a little bit?]
>
>
>
> David,
>
>
>
> Yes, I agree that it would be great if we can upstream the changes. I
> think that's possible, and I think we should try before making the decision
> to maintain our own fork.
>
>
>
> Yes, I also agree that the current config fixes a bunch of things, but
> also messes up a lot of other things too.
>
>
>
> Re: # indenting. I am not sure I understand your comments about this. Are
> the NuttX Style Guide sections on indenting
> <https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#indentation>
> correct? If so, we can probably add clang-format options to support this
> style. If not, can we correct the Style Guide?
>
> [DBS] The correct thing to do is indent on nested #, but we have to ignore
> the #if defined (THIS FILE) So the next # is at Col1.
>
>
>
> This is the convention for thew old way of doing #pragma once
>
>
>
> #if !defined(this file)
>
> #define this file
>
>
>
> #define FOO
>
>
>
> #endif /* !defined(this file) */
>
>
>
> This is what you would infer from the Style Guide, but now looking at it I
> see it has been updated
>
>
>
> #if !defined(this file)
>
> #  define this file
>
>
>
> #  define FOO
>
>
>
> #endif /* !defined(this file) */
>
>
>
>
>
> Re: clang-format having a greater ROI, I think that's true if we can get
> it to work. It has a big community supporting it.
>
>
>
> I will create a PR for the .clang-format and a README, and mark it [Do Not
> Merge] and DRAFT PR. Do you also want me to include the formatted files
> under sched/?
>
> [DBS] Yep all of it. So we can review it and play with it all.
>
>
>
> cheers
>
> adam
>
>
>
>
>
> On Tue, Mar 17, 2020 at 5:56 AM David Sidrane 
> wrote:
>
> Adam,
>
>
>
> Thank you for putting in the effort on this!
>
>
>
> I only suggest forking incase the LLVM project finds Nuttx' CS not of
> value. It would be great if we can upstream the changes.
>
>
>
> If we make this a DNM (Do Not Merge) Pull request we can discuss in-line
> where the comments will have context. Just put [Do Not Merge] in the title
> and Leave it set at DRAFT PR.
>
>
>
> Hopefully everyone will understand that we are doing this to collaborate
> and add valuable feedback.
>
>
>
> My overall view is it fixed a bunch of stuff NxStyle does not even detect.
> But it messes up a bunch of things too.
>
>
>
> One issue is the # indenting. Since Nuttx does not allow the modern use of
> `#pragma once` the guard #if defs set the indent to 1 to start with. I
> remember getting lambasted for doing this after reading the CS and
> following it. So we will need a -nuttx-option to set the indent to -1 and
> then only indent for > 0.
>
>
>
> Let's continue to take it apart in the PR and see what the level of effort
> would be to get it in shape.
>
>
>
> Personally, I am not afflicted with NIH syndrome, I value others quality
> tools, and I would rather put the effort in to this mature tool than
> Nxstyle, as the ROI will be much, much greater.
>
>
>
> David
>
>
>
>
>
>
>
> *From:* Adam Feuer [mailto:a...@starcat.io]
> *Sent:* Monday, March 16, 2020 3:35 PM
> *To:* David Sidrane; dev@nuttx.apache.org
> *Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
> precheck a little bit?]
>
>
>
> Here's a Github compare with .clang-format file and the results when
> clang-format-9 is run on all the files under sched/:
>
>
>
>
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched
>
>
>
> If anyone has comments or observations I would love to know them.
>
>
>
> -adam
>
>
>
> On Mon, Mar 16, 2020 at 3:23 PM Adam Feuer  wrote:
>
> David,
>
>
>
> It would be a --style=NuttX option on the command line, and a set of
> configurations. But yes that might be the way to go. I don't think forking
> it would be the right thing, if we can modify it to do what we want, we
> should be able to add options that are configurable, submit PRs and get
> them merged. Then the features can be used an

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Gregory Nutt]

Re: # indenting. I am not sure I understand your comments about this. Are
the NuttX Style Guide sections on indenting

correct? If so, we can probably add clang-format options to support this
style. If not, can we correct the Style Guide?

[DBS] The correct thing to do is indent on nested #, but we have to ignore
the #if defined (THIS FILE) So the next # is at Col1.
You also have to ignore # indentation in the code sections of the 
files.  They are not indented because the indentation interferes with C 
indentation and makes the files unreadable.  Per coding standard.

RE: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[David Sidrane]

Hi Adam,



See inline…



*From:* Adam Feuer [mailto:a...@starcat.io]
*Sent:* Tuesday, March 17, 2020 12:10 PM
*To:* David Sidrane
*Cc:* dev@nuttx.apache.org
*Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
precheck a little bit?]



David,



Yes, I agree that it would be great if we can upstream the changes. I think
that's possible, and I think we should try before making the decision to
maintain our own fork.



Yes, I also agree that the current config fixes a bunch of things, but also
messes up a lot of other things too.



Re: # indenting. I am not sure I understand your comments about this. Are
the NuttX Style Guide sections on indenting
<https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#indentation>
correct? If so, we can probably add clang-format options to support this
style. If not, can we correct the Style Guide?

[DBS] The correct thing to do is indent on nested #, but we have to ignore
the #if defined (THIS FILE) So the next # is at Col1.



This is the convention for thew old way of doing #pragma once



#if !defined(this file)

#define this file



#define FOO



#endif /* !defined(this file) */



This is what you would infer from the Style Guide, but now looking at it I
see it has been updated



#if !defined(this file)

#  define this file



#  define FOO



#endif /* !defined(this file) */





Re: clang-format having a greater ROI, I think that's true if we can get it
to work. It has a big community supporting it.



I will create a PR for the .clang-format and a README, and mark it [Do Not
Merge] and DRAFT PR. Do you also want me to include the formatted files
under sched/?

[DBS] Yep all of it. So we can review it and play with it all.



cheers

adam





On Tue, Mar 17, 2020 at 5:56 AM David Sidrane 
wrote:

Adam,



Thank you for putting in the effort on this!



I only suggest forking incase the LLVM project finds Nuttx' CS not of
value. It would be great if we can upstream the changes.



If we make this a DNM (Do Not Merge) Pull request we can discuss in-line
where the comments will have context. Just put [Do Not Merge] in the title
and Leave it set at DRAFT PR.



Hopefully everyone will understand that we are doing this to collaborate
and add valuable feedback.



My overall view is it fixed a bunch of stuff NxStyle does not even detect.
But it messes up a bunch of things too.



One issue is the # indenting. Since Nuttx does not allow the modern use of
`#pragma once` the guard #if defs set the indent to 1 to start with. I
remember getting lambasted for doing this after reading the CS and
following it. So we will need a -nuttx-option to set the indent to -1 and
then only indent for > 0.



Let's continue to take it apart in the PR and see what the level of effort
would be to get it in shape.



Personally, I am not afflicted with NIH syndrome, I value others quality
tools, and I would rather put the effort in to this mature tool than
Nxstyle, as the ROI will be much, much greater.



David







*From:* Adam Feuer [mailto:a...@starcat.io]
*Sent:* Monday, March 16, 2020 3:35 PM
*To:* David Sidrane; dev@nuttx.apache.org
*Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
precheck a little bit?]



Here's a Github compare with .clang-format file and the results when
clang-format-9 is run on all the files under sched/:



https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched



If anyone has comments or observations I would love to know them.



-adam



On Mon, Mar 16, 2020 at 3:23 PM Adam Feuer  wrote:

David,



It would be a --style=NuttX option on the command line, and a set of
configurations. But yes that might be the way to go. I don't think forking
it would be the right thing, if we can modify it to do what we want, we
should be able to add options that are configurable, submit PRs and get
them merged. Then the features can be used and maintained by everyone who
uses clang-format, a very large userbase, that would be a very good option.



I think it can work... I'll post link to what it can currently do in sched/
so we can look at the differences to see how close we are.



-adam



On Sun, Mar 15, 2020 at 6:49 AM David Sidrane 
wrote:

Hi Adam,



Hmmm…Would the shortest path, on this issue, be to add some -NuttX options
and submit a PR to upstream or Fork it?



It may be that time spent training Clang-format is better than time spent
on nstyle, as it is a "shorter haul" with a much, much greater return.



David



*From:* Adam Feuer [mailto:a...@starcat.io]
*Sent:* Saturday, March 14, 2020 1:59 PM
*To:* dev@nuttx.apache.org; david.sidr...@nscdg.com; w8j...@gmail.com
*Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
precheck a little bit?]



I looked at the clang-format source code. It has trouble aligning the = of
a -= or +=. I easily made a change to align on the - or + of a -= or +=,
but some work will be necessary to give an

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

David,

Yes, I agree that it would be great if we can upstream the changes. I think
that's possible, and I think we should try before making the decision to
maintain our own fork.

Yes, I also agree that the current config fixes a bunch of things, but also
messes up a lot of other things too.

Re: # indenting. I am not sure I understand your comments about this. Are
the NuttX Style Guide sections on indenting
<https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#indentation>
correct? If so, we can probably add clang-format options to support this
style. If not, can we correct the Style Guide?

Re: clang-format having a greater ROI, I think that's true if we can get it
to work. It has a big community supporting it.

I will create a PR for the .clang-format and a README, and mark it [Do Not
Merge] and DRAFT PR. Do you also want me to include the formatted files
under sched/?

cheers
adam


On Tue, Mar 17, 2020 at 5:56 AM David Sidrane 
wrote:

> Adam,
>
>
>
> Thank you for putting in the effort on this!
>
>
>
> I only suggest forking incase the LLVM project finds Nuttx' CS not of
> value. It would be great if we can upstream the changes.
>
>
>
> If we make this a DNM (Do Not Merge) Pull request we can discuss in-line
> where the comments will have context. Just put [Do Not Merge] in the title
> and Leave it set at DRAFT PR.
>
>
>
> Hopefully everyone will understand that we are doing this to collaborate
> and add valuable feedback.
>
>
>
> My overall view is it fixed a bunch of stuff NxStyle does not even detect.
> But it messes up a bunch of things too.
>
>
>
> One issue is the # indenting. Since Nuttx does not allow the modern use of
> `#pragma once` the guard #if defs set the indent to 1 to start with. I
> remember getting lambasted for doing this after reading the CS and
> following it. So we will need a -nuttx-option to set the indent to -1 and
> then only indent for > 0.
>
>
>
> Let's continue to take it apart in the PR and see what the level of effort
> would be to get it in shape.
>
>
>
> Personally, I am not afflicted with NIH syndrome, I value others quality
> tools, and I would rather put the effort in to this mature tool than
> Nxstyle, as the ROI will be much, much greater.
>
>
>
> David
>
>
>
>
>
>
>
> *From:* Adam Feuer [mailto:a...@starcat.io]
> *Sent:* Monday, March 16, 2020 3:35 PM
> *To:* David Sidrane; dev@nuttx.apache.org
> *Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
> precheck a little bit?]
>
>
>
> Here's a Github compare with .clang-format file and the results when
> clang-format-9 is run on all the files under sched/:
>
>
>
>
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched
>
>
>
> If anyone has comments or observations I would love to know them.
>
>
>
> -adam
>
>
>
> On Mon, Mar 16, 2020 at 3:23 PM Adam Feuer  wrote:
>
> David,
>
>
>
> It would be a --style=NuttX option on the command line, and a set of
> configurations. But yes that might be the way to go. I don't think forking
> it would be the right thing, if we can modify it to do what we want, we
> should be able to add options that are configurable, submit PRs and get
> them merged. Then the features can be used and maintained by everyone who
> uses clang-format, a very large userbase, that would be a very good option.
>
>
>
> I think it can work... I'll post link to what it can currently do in
> sched/ so we can look at the differences to see how close we are.
>
>
>
> -adam
>
>
>
> On Sun, Mar 15, 2020 at 6:49 AM David Sidrane 
> wrote:
>
> Hi Adam,
>
>
>
> Hmmm…Would the shortest path, on this issue, be to add some -NuttX options
> and submit a PR to upstream or Fork it?
>
>
>
> It may be that time spent training Clang-format is better than time spent
> on nstyle, as it is a "shorter haul" with a much, much greater return.
>
>
>
> David
>
>
>
> *From:* Adam Feuer [mailto:a...@starcat.io]
> *Sent:* Saturday, March 14, 2020 1:59 PM
> *To:* dev@nuttx.apache.org; david.sidr...@nscdg.com; w8j...@gmail.com
> *Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
> precheck a little bit?]
>
>
>
> I looked at the clang-format source code. It has trouble aligning the = of
> a -= or +=. I easily made a change to align on the - or + of a -= or +=,
> but some work will be necessary to give an option that aligns the way the
> nuttx code does it. Will think more about it.
>
>
>
> -adam
>
>
>
>
>
> On Sat, Mar 14, 2020 at 1:21 PM Adam Feuer  wrote:
>
> David, Maciej, Pete

RE: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[David Sidrane]

Adam,



Thank you for putting in the effort on this!



I only suggest forking incase the LLVM project finds Nuttx' CS not of
value. It would be great if we can upstream the changes.



If we make this a DNM (Do Not Merge) Pull request we can discuss in-line
where the comments will have context. Just put [Do Not Merge] in the title
and Leave it set at DRAFT PR.



Hopefully everyone will understand that we are doing this to collaborate
and add valuable feedback.



My overall view is it fixed a bunch of stuff NxStyle does not even detect.
But it messes up a bunch of things too.



One issue is the # indenting. Since Nuttx does not allow the modern use of
`#pragma once` the guard #if defs set the indent to 1 to start with. I
remember getting lambasted for doing this after reading the CS and
following it. So we will need a -nuttx-option to set the indent to -1 and
then only indent for > 0.



Let's continue to take it apart in the PR and see what the level of effort
would be to get it in shape.



Personally, I am not afflicted with NIH syndrome, I value others quality
tools, and I would rather put the effort in to this mature tool than
Nxstyle, as the ROI will be much, much greater.



David







*From:* Adam Feuer [mailto:a...@starcat.io]
*Sent:* Monday, March 16, 2020 3:35 PM
*To:* David Sidrane; dev@nuttx.apache.org
*Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
precheck a little bit?]



Here's a Github compare with .clang-format file and the results when
clang-format-9 is run on all the files under sched/:



https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched



If anyone has comments or observations I would love to know them.



-adam



On Mon, Mar 16, 2020 at 3:23 PM Adam Feuer  wrote:

David,



It would be a --style=NuttX option on the command line, and a set of
configurations. But yes that might be the way to go. I don't think forking
it would be the right thing, if we can modify it to do what we want, we
should be able to add options that are configurable, submit PRs and get
them merged. Then the features can be used and maintained by everyone who
uses clang-format, a very large userbase, that would be a very good option.



I think it can work... I'll post link to what it can currently do in sched/
so we can look at the differences to see how close we are.



-adam



On Sun, Mar 15, 2020 at 6:49 AM David Sidrane 
wrote:

Hi Adam,



Hmmm…Would the shortest path, on this issue, be to add some -NuttX options
and submit a PR to upstream or Fork it?



It may be that time spent training Clang-format is better than time spent
on nstyle, as it is a "shorter haul" with a much, much greater return.



David



*From:* Adam Feuer [mailto:a...@starcat.io]
*Sent:* Saturday, March 14, 2020 1:59 PM
*To:* dev@nuttx.apache.org; david.sidr...@nscdg.com; w8j...@gmail.com
*Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
precheck a little bit?]



I looked at the clang-format source code. It has trouble aligning the = of
a -= or +=. I easily made a change to align on the - or + of a -= or +=,
but some work will be necessary to give an option that aligns the way the
nuttx code does it. Will think more about it.



-adam





On Sat, Mar 14, 2020 at 1:21 PM Adam Feuer  wrote:

David, Maciej, Peter,



Thanks for your help!



IndentPPDirectives: PPDIS_AfterHash works. The actual syntax for the
.clang-format file is this:



IndentPPDirectives: AfterHash



That makes things a lot better. There are a bunch of inconsistent macro
indents under sched/ though— many macros indent ok, but there are a bunch
that don't. Not sure what to do about that... are they really inconsistent?
If so maybe we make a small PR that fixes the inconsistent indents?



What seems to be next:

·  alignment of successive expressions

   reltime.tv_nsec += NSEC_PER_SEC;
-  reltime.tv_sec  -= 1;
+  reltime.tv_sec -= 1;

·  alignment of comment blocks ­— to make sure they line up with the next
comment line in the block

For instance:



-  /* The resulting set is the intersection of the current set and
+/* The resulting set is the intersection of the current set and
* the complement of the signal set pointed to by _set.
*/

·  evaluating inconsistencies in the alignment style... some expressions
and declarations are aligned, others are not... I need to consult the style
guide to see what it says.



I'm using clang-format-9. Here's the command lines I'm running to generate
and look at the changes (in the nutt/ dir):



$ find ./sched/ -iname "*.h" -or -iname "*.c" | xargs clang-format-9 -i
-style=file

$ git diff

$ # change .clang-format

$ git stash; git stash drop





Here's my .clang format file as of now:



---
Language:Cpp
AccessModifierOffset: -2
AlignAfterOpenBracket: Align
AlignConsecutiveMacros: true
AlignConsecutiveAssignments: true
A

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Gregory Nutt]

Yes, it may be horrible. Please don't look at the results right now then...
this is a work in progress.


Okay.  Thanks for clarifying that.  nxstyle is still your friend.

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Gregory Nutt]

 ...  It is simply not following the NuttX coding standard.

Perhaps what you should do is also run the modified files through 
nxstyle.  nxstyle, like most tools, is imperfect but it is the yardstick 
we use to evaluating conformance to the coding standard now.  Certainly 
a first step would be to run your tool then run nxstyle immediately 
afterward.  nxstyle should not detect any problems if you are on the 
right track (currently, I think nxstyle would generate reams of 
complaints about the output).


If the generated code cannot make it through nxstyle, then it is not 
ready to be reviewed or considered in anyway.


Making it through nxstyle does not mean that the tools is working 
correctly, however.  There are many things not checked by nxstyle, but 
this would give you a starting point for getting these huge issues out 
of the way.  That would then be an appropriate time to review the 
remaining diffs.


Greg

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

Hi Greg,

Yes, it may be horrible. Please don't look at the results right now then...
this is a work in progress.

I'm just trying to do the development and coordination in the open, as the
Apache folks have asked— no off-list communication, right?

I just posted these so David and others could easily look at the output,
not because I was going to submit any kind of PR. It seems like it should
be ok to show work-in-progress here. Otherwise how can we collaborate?

When it's more fully baked, we'll let you know. :)

-adam

On Mon, Mar 16, 2020 at 4:36 PM Gregory Nutt  wrote:

> I mentioned in the previous email that the tools was screwing up
> vertifcal alignment is places where it is recommend and in places where
> is it required.  Here is an example of the later:
>
>   * sched/irq/irq.h
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-08538ebd3dc53f5f85f131c24c84be9d
> >,
> beginning at line 75 all veritical alignment of right hand comments
> lost.
>
> A few more:
>
>   * clock_timespec_subtract.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-906c272ac7c0b95506056c0c431ab276
> >,
> insanity at line 69.  Similar nonsense at group_setuid.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-f39b80ac9e4440e2d2889ef55425f3ef
> >
> line 76, group_signal.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-750c9acf01eefd4c69e3186ceb9519bd
> >
> , line 90 and other places
>   * nx_smpstart.c,
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-8195c6cbcb792a1f2e6b337039a1d111
> >line
> 120, removes required spacing after semi-colon
>   * It seems to screw up the indenttaion after any conditional logic.
> Like nx_bringup.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-9aa86408017fde10647798c88d4c62b0
> >,
> line 79-110
>   * group_malloc.c,
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-6cb24cb106c424372fc4dd44e55b8a07
> >line
> 51, screw sup indentation of conditional.  also group_zalloc.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-87884d55782f00d26a012863add30407
> >,
> line 47 and other places
>   * group_setupidlefiles.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-aa3bb2f5a7f4db9b96218e96098ea53c
> >,
> line 118 more destruction of indentation.  Also nx_bringup.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-9aa86408017fde10647798c88d4c62b0
> >,
> line 74 and other places
>   * nx_start.c
> <
> https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched#diff-e36621b38d26a5eeb9f0c9c779931e7b
> >
> , all braces trashed beginning at line 253.  Right alignment of all
> comments lost.
>
> I can't look at any more.  Occasionally I see a new error and suspect
> that there are more.  But these I have describe account for at least
> most.  I see these kinds of errors in 100's perhaps 1000's of places.
> Lots of indentation and alignment errors.  It is simply not following
> the NuttX coding standard.
>
>
>

-- 
Adam Feuer 

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Gregory Nutt]

I mentioned in the previous email that the tools was screwing up 
vertifcal alignment is places where it is recommend and in places where 
is it required.  Here is an example of the later:


 * sched/irq/irq.h
   
,
   beginning at line 75 all veritical alignment of right hand comments
   lost.

A few more:

 * clock_timespec_subtract.c
   
,
   insanity at line 69.  Similar nonsense at group_setuid.c
   

   line 76, group_signal.c
   

   , line 90 and other places
 * nx_smpstart.c,
   
line
   120, removes required spacing after semi-colon
 * It seems to screw up the indenttaion after any conditional logic. 
   Like nx_bringup.c
   
,
   line 79-110
 * group_malloc.c,
   
line
   51, screw sup indentation of conditional.  also group_zalloc.c
   
,
   line 47 and other places
 * group_setupidlefiles.c
   
,
   line 118 more destruction of indentation.  Also nx_bringup.c
   
,
   line 74 and other places
 * nx_start.c
   

   , all braces trashed beginning at line 253.  Right alignment of all
   comments lost.

I can't look at any more.  Occasionally I see a new error and suspect 
that there are more.  But these I have describe account for at least 
most.  I see these kinds of errors in 100's perhaps 1000's of places.  
Lots of indentation and alignment errors.  It is simply not following 
the NuttX coding standard.

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Gregory Nutt]

Here's a Github compare with .clang-format file and the results when
clang-format-9 is run on all the files under sched/:

https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched

If anyone has comments or observations I would love to know them.


That is horrible!  Not even close.  It is doing massive corruption of 
the original files.  I appreciate that this is a work in progress, but 
it is not close enough yet for any kind of review.  At present, it just 
does massive corruption of files.


 * It is destroying indentation randomly (like line 71 of clock.h).
 * It is destroying the careful alignment of prototypes like line 90
   and beyond.
 * It removes all vertical alignment, some optional and some required.
 * This is crazy.  It moved a left bracket onto the same line as a
   definition (line 54 of clock_dow.c)
 * in clock_systimer.c, pre-processor definitions must not be indented
   when use in the code section (numerous places).
   https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#indentation
 * in clock_systimespec.c, line 134.  The indentation was correct.  The
   tool ruined it.  Also line 142 it again destroys the proper
   alignment of the first line of C comment blocks. Numerous other places.
 * in clock_timekeeping.c, line 88 abd line 105 spurious destruction of
   vertical alignment.
 * And on an on..  There is too much there to review.

We agreed when we started this that the success criteria was no 
differences from the original.  I could see compromising a few find 
details, but this is not close.  This is the worst formatting 
programming I have seen.  All of the formatters under tools are better 
and none of them are acceptable.


I don't know if these can be fixed or not but please do not commit 
anything now.  This is not ready.  It is just too wrong and I would not 
want anyone to use it.

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

Here's a Github compare with .clang-format file and the results when
clang-format-9 is run on all the files under sched/:

https://github.com/apache/incubator-nuttx/compare/master...starcat-io:explore/clang-format-sched

If anyone has comments or observations I would love to know them.

-adam

On Mon, Mar 16, 2020 at 3:23 PM Adam Feuer  wrote:

> David,
>
> It would be a --style=NuttX option on the command line, and a set of
> configurations. But yes that might be the way to go. I don't think forking
> it would be the right thing, if we can modify it to do what we want, we
> should be able to add options that are configurable, submit PRs and get
> them merged. Then the features can be used and maintained by everyone who
> uses clang-format, a very large userbase, that would be a very good option.
>
> I think it can work... I'll post link to what it can currently do in
> sched/ so we can look at the differences to see how close we are.
>
> -adam
>
> On Sun, Mar 15, 2020 at 6:49 AM David Sidrane 
> wrote:
>
>> Hi Adam,
>>
>>
>>
>> Hmmm…Would the shortest path, on this issue, be to add some -NuttX
>> options and submit a PR to upstream or Fork it?
>>
>>
>>
>> It may be that time spent training Clang-format is better than time spent
>> on nstyle, as it is a "shorter haul" with a much, much greater return.
>>
>>
>>
>> David
>>
>>
>>
>> *From:* Adam Feuer [mailto:a...@starcat.io]
>> *Sent:* Saturday, March 14, 2020 1:59 PM
>> *To:* dev@nuttx.apache.org; david.sidr...@nscdg.com; w8j...@gmail.com
>> *Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
>> precheck a little bit?]
>>
>>
>>
>> I looked at the clang-format source code. It has trouble aligning the =
>> of a -= or +=. I easily made a change to align on the - or + of a -= or +=,
>> but some work will be necessary to give an option that aligns the way the
>> nuttx code does it. Will think more about it.
>>
>>
>>
>> -adam
>>
>>
>>
>>
>>
>> On Sat, Mar 14, 2020 at 1:21 PM Adam Feuer  wrote:
>>
>> David, Maciej, Peter,
>>
>>
>>
>> Thanks for your help!
>>
>>
>>
>> IndentPPDirectives: PPDIS_AfterHash works. The actual syntax for the
>> .clang-format file is this:
>>
>>
>>
>> IndentPPDirectives: AfterHash
>>
>>
>>
>> That makes things a lot better. There are a bunch of inconsistent macro
>> indents under sched/ though— many macros indent ok, but there are a bunch
>> that don't. Not sure what to do about that... are they really inconsistent?
>> If so maybe we make a small PR that fixes the inconsistent indents?
>>
>>
>>
>> What seems to be next:
>>
>> ·  alignment of successive expressions
>>
>>reltime.tv_nsec += NSEC_PER_SEC;
>> -  reltime.tv_sec  -= 1;
>> +  reltime.tv_sec -= 1;
>>
>> ·  alignment of comment blocks ­— to make sure they line up with the
>> next comment line in the block
>>
>> For instance:
>>
>>
>>
>> -  /* The resulting set is the intersection of the current set and
>> +/* The resulting set is the intersection of the current set
>> and
>> * the complement of the signal set pointed to by _set.
>> */
>>
>> ·  evaluating inconsistencies in the alignment style... some expressions
>> and declarations are aligned, others are not... I need to consult the style
>> guide to see what it says.
>>
>>
>>
>> I'm using clang-format-9. Here's the command lines I'm running to
>> generate and look at the changes (in the nutt/ dir):
>>
>>
>>
>> $ find ./sched/ -iname "*.h" -or -iname "*.c" | xargs clang-format-9 -i
>> -style=file
>>
>> $ git diff
>>
>> $ # change .clang-format
>>
>> $ git stash; git stash drop
>>
>> 
>>
>>
>>
>> Here's my .clang format file as of now:
>>
>>
>>
>> ---
>> Language:Cpp
>> AccessModifierOffset: -2
>> AlignAfterOpenBracket: Align
>> AlignConsecutiveMacros: true
>> AlignConsecutiveAssignments: true
>> AlignConsecutiveDeclarations: true
>> AlignEscapedNewlines: DontAlign
>> AlignOperands:   true
>> AlignTrailingComments: true
>> AllowAllArgumentsOnNextLine: true
>> AllowAllConstructorInitializersOnNextLine: true
>> AllowAllParametersOfDeclarationOnNextLine: true
>> AllowShortBlocksOnASingleLine: false
&

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

David,

It would be a --style=NuttX option on the command line, and a set of
configurations. But yes that might be the way to go. I don't think forking
it would be the right thing, if we can modify it to do what we want, we
should be able to add options that are configurable, submit PRs and get
them merged. Then the features can be used and maintained by everyone who
uses clang-format, a very large userbase, that would be a very good option.

I think it can work... I'll post link to what it can currently do in sched/
so we can look at the differences to see how close we are.

-adam

On Sun, Mar 15, 2020 at 6:49 AM David Sidrane 
wrote:

> Hi Adam,
>
>
>
> Hmmm…Would the shortest path, on this issue, be to add some -NuttX options
> and submit a PR to upstream or Fork it?
>
>
>
> It may be that time spent training Clang-format is better than time spent
> on nstyle, as it is a "shorter haul" with a much, much greater return.
>
>
>
> David
>
>
>
> *From:* Adam Feuer [mailto:a...@starcat.io]
> *Sent:* Saturday, March 14, 2020 1:59 PM
> *To:* dev@nuttx.apache.org; david.sidr...@nscdg.com; w8j...@gmail.com
> *Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
> precheck a little bit?]
>
>
>
> I looked at the clang-format source code. It has trouble aligning the = of
> a -= or +=. I easily made a change to align on the - or + of a -= or +=,
> but some work will be necessary to give an option that aligns the way the
> nuttx code does it. Will think more about it.
>
>
>
> -adam
>
>
>
>
>
> On Sat, Mar 14, 2020 at 1:21 PM Adam Feuer  wrote:
>
> David, Maciej, Peter,
>
>
>
> Thanks for your help!
>
>
>
> IndentPPDirectives: PPDIS_AfterHash works. The actual syntax for the
> .clang-format file is this:
>
>
>
> IndentPPDirectives: AfterHash
>
>
>
> That makes things a lot better. There are a bunch of inconsistent macro
> indents under sched/ though— many macros indent ok, but there are a bunch
> that don't. Not sure what to do about that... are they really inconsistent?
> If so maybe we make a small PR that fixes the inconsistent indents?
>
>
>
> What seems to be next:
>
> ·  alignment of successive expressions
>
>reltime.tv_nsec += NSEC_PER_SEC;
> -  reltime.tv_sec  -= 1;
> +  reltime.tv_sec -= 1;
>
> ·  alignment of comment blocks ­— to make sure they line up with the next
> comment line in the block
>
> For instance:
>
>
>
> -  /* The resulting set is the intersection of the current set and
> +/* The resulting set is the intersection of the current set
> and
> * the complement of the signal set pointed to by _set.
> */
>
> ·  evaluating inconsistencies in the alignment style... some expressions
> and declarations are aligned, others are not... I need to consult the style
> guide to see what it says.
>
>
>
> I'm using clang-format-9. Here's the command lines I'm running to generate
> and look at the changes (in the nutt/ dir):
>
>
>
> $ find ./sched/ -iname "*.h" -or -iname "*.c" | xargs clang-format-9 -i
> -style=file
>
> $ git diff
>
> $ # change .clang-format
>
> $ git stash; git stash drop
>
> 
>
>
>
> Here's my .clang format file as of now:
>
>
>
> ---
> Language:Cpp
> AccessModifierOffset: -2
> AlignAfterOpenBracket: Align
> AlignConsecutiveMacros: true
> AlignConsecutiveAssignments: true
> AlignConsecutiveDeclarations: true
> AlignEscapedNewlines: DontAlign
> AlignOperands:   true
> AlignTrailingComments: true
> AllowAllArgumentsOnNextLine: true
> AllowAllConstructorInitializersOnNextLine: true
> AllowAllParametersOfDeclarationOnNextLine: true
> AllowShortBlocksOnASingleLine: false
> AllowShortCaseLabelsOnASingleLine: false
> AllowShortFunctionsOnASingleLine: All
> AllowShortLambdasOnASingleLine: All
> AllowShortIfStatementsOnASingleLine: Never
> AllowShortLoopsOnASingleLine: false
> AlwaysBreakAfterDefinitionReturnType: None
> AlwaysBreakAfterReturnType: None
> AlwaysBreakBeforeMultilineStrings: false
> AlwaysBreakTemplateDeclarations: MultiLine
> BinPackArguments: true
> BinPackParameters: true
> BraceWrapping:
>   AfterCaseLabel:  false
>   AfterClass:  false
>   AfterControlStatement: true
>   AfterEnum:   true
>   AfterFunction:   true
>   AfterNamespace:  false
>   AfterObjCDeclaration: false
>   AfterStruct: true
>   AfterUnion:  true
>   AfterExternBlock: false
>   BeforeCatch: false
>   BeforeElse:  true
>   IndentBraces:true
>   SplitEmptyFunction: true
>   SplitEmptyRecord: true
>   

Re: Should we relax precheck a little bit?

[Schock, Johannes - NIVUS GmbH]

Part of the discussion on whitelisting for nxstyle went on on the Github PR:
https://github.com/apache/incubator-nuttx/pull/569

To draw a conclusion from my side, I withdraw my supposal, because of "no 
metadata in source files".
I will have a look at the .nxignore solution and if I find time I will file a 
PR.
But this can happen the next weekend the earliest. So if one would like to take 
over this task earlier, please give notice.


-Ursprüngliche Nachricht-
Von: Gregory Nutt [mailto:spudan...@gmail.com] 
Gesendet: Sonntag, 15. März 2020 14:37
An: dev@nuttx.apache.org
Betreff: Re: Should we relax precheck a little bit?


> Yeah I thought about the too
>
> But think of the can of worm on the commit history: Every PR touches the cfg
> file.

I don't see that.  Modifications to the white list file, whatever it is 
called, would be very rare.  Only when a new non-compliant name is added 
which should be never.

We should not permit people to create new non-compliant names. The 
existing non-compliant names are a consequence of names defined in other 
standards and by chip manufacturers.  If POSIX changes or if we add 
support for new chips, then and only then would the whitelists change.

I see no can.  I see no worms.

RE: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[David Sidrane]

Hi Adam,



Hmmm…Would the shortest path, on this issue, be to add some -NuttX options
and submit a PR to upstream or Fork it?



It may be that time spent training Clang-format is better than time spent
on nstyle, as it is a "shorter haul" with a much, much greater return.



David



*From:* Adam Feuer [mailto:a...@starcat.io]
*Sent:* Saturday, March 14, 2020 1:59 PM
*To:* dev@nuttx.apache.org; david.sidr...@nscdg.com; w8j...@gmail.com
*Subject:* Re: Nuttx Code Formatter Progress [Was RE: Should we relax
precheck a little bit?]



I looked at the clang-format source code. It has trouble aligning the = of
a -= or +=. I easily made a change to align on the - or + of a -= or +=,
but some work will be necessary to give an option that aligns the way the
nuttx code does it. Will think more about it.



-adam





On Sat, Mar 14, 2020 at 1:21 PM Adam Feuer  wrote:

David, Maciej, Peter,



Thanks for your help!



IndentPPDirectives: PPDIS_AfterHash works. The actual syntax for the
.clang-format file is this:



IndentPPDirectives: AfterHash



That makes things a lot better. There are a bunch of inconsistent macro
indents under sched/ though— many macros indent ok, but there are a bunch
that don't. Not sure what to do about that... are they really inconsistent?
If so maybe we make a small PR that fixes the inconsistent indents?



What seems to be next:

·  alignment of successive expressions

   reltime.tv_nsec += NSEC_PER_SEC;
-  reltime.tv_sec  -= 1;
+  reltime.tv_sec -= 1;

·  alignment of comment blocks ­— to make sure they line up with the next
comment line in the block

For instance:



-  /* The resulting set is the intersection of the current set and
+/* The resulting set is the intersection of the current set and
* the complement of the signal set pointed to by _set.
*/

·  evaluating inconsistencies in the alignment style... some expressions
and declarations are aligned, others are not... I need to consult the style
guide to see what it says.



I'm using clang-format-9. Here's the command lines I'm running to generate
and look at the changes (in the nutt/ dir):



$ find ./sched/ -iname "*.h" -or -iname "*.c" | xargs clang-format-9 -i
-style=file

$ git diff

$ # change .clang-format

$ git stash; git stash drop





Here's my .clang format file as of now:



---
Language:Cpp
AccessModifierOffset: -2
AlignAfterOpenBracket: Align
AlignConsecutiveMacros: true
AlignConsecutiveAssignments: true
AlignConsecutiveDeclarations: true
AlignEscapedNewlines: DontAlign
AlignOperands:   true
AlignTrailingComments: true
AllowAllArgumentsOnNextLine: true
AllowAllConstructorInitializersOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: false
AllowShortCaseLabelsOnASingleLine: false
AllowShortFunctionsOnASingleLine: All
AllowShortLambdasOnASingleLine: All
AllowShortIfStatementsOnASingleLine: Never
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: None
AlwaysBreakBeforeMultilineStrings: false
AlwaysBreakTemplateDeclarations: MultiLine
BinPackArguments: true
BinPackParameters: true
BraceWrapping:
  AfterCaseLabel:  false
  AfterClass:  false
  AfterControlStatement: true
  AfterEnum:   true
  AfterFunction:   true
  AfterNamespace:  false
  AfterObjCDeclaration: false
  AfterStruct: true
  AfterUnion:  true
  AfterExternBlock: false
  BeforeCatch: false
  BeforeElse:  true
  IndentBraces:true
  SplitEmptyFunction: true
  SplitEmptyRecord: true
  SplitEmptyNamespace: true
BreakBeforeBinaryOperators: None
BreakBeforeBraces: Custom
BreakBeforeInheritanceComma: false
BreakInheritanceList: BeforeColon
BreakBeforeTernaryOperators: true
BreakConstructorInitializersBeforeComma: false
BreakConstructorInitializers: BeforeColon
BreakAfterJavaFieldAnnotations: false
BreakStringLiterals: true
ColumnLimit: 0
CommentPragmas:  '^ IWYU pragma:'
CompactNamespaces: false
ConstructorInitializerAllOnOneLineOrOnePerLine: false
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 0
Cpp11BracedListStyle: false
DerivePointerAlignment: false
DisableFormat:   false
ExperimentalAutoDetectBinPacking: false
FixNamespaceComments: false
ForEachMacros:
  - foreach
  - Q_FOREACH
  - BOOST_FOREACH
IncludeBlocks:   Preserve
IncludeCategories:
  - Regex:   '^"(llvm|llvm-c|clang|clang-c)/'
Priority:2
  - Regex:   '^(<|"(gtest|gmock|isl|json)/)'
Priority:3
  - Regex:   '.*'
Priority:1
IncludeIsMainRegex: '(Test)?$'
IndentCaseLabels: true
IndentPPDirectives: AfterHash
IndentWidth: 2
IndentWrappedFunctionNames: false
JavaScriptQuotes: Leave
JavaScriptWrapImports: true
KeepEmptyLinesAtTheStartOfBlocks: true
MacroBlockBegin: ''
MacroBlockEnd:   ''
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
ObjCBinPackProtocolList: Auto
ObjCBlockI

Re: Should we relax precheck a little bit?

[Gregory Nutt]

Yeah I thought about the too

But think of the can of worm on the commit history: Every PR touches the cfg
file.


I don't see that.  Modifications to the white list file, whatever it is 
called, would be very rare.  Only when a new non-compliant name is added 
which should be never.


We should not permit people to create new non-compliant names. The 
existing non-compliant names are a consequence of names defined in other 
standards and by chip manufacturers.  If POSIX changes or if we add 
support for new chips, then and only then would the whitelists change.


I see no can.  I see no worms.

Re: Should we relax precheck a little bit?

[Gregory Nutt]

How about we put this special treatment to another file(e.g.
nxstyle.cfg)? So we don't pollute the source code.


+1 I like it.  I just suggested basically the same think in the PR.  I 
suggested the name .nxignore.


If a .nxignore file exists in the same directory as the file under test, 
then it could be read to get the white list.  Perhaps there would be a 
global .nxignore file at the top level, or under tools/?


Greg

RE: Should we relax precheck a little bit?

[David Sidrane]

Yeah I thought about the too

But think of the can of worm on the commit history: Every PR touches the cfg
file.

Rebase HELL! Yuch!

Fix or replace the tool!

-Original Message-
From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
Sent: Sunday, March 15, 2020 6:08 AM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

How about we put this special treatment to another file(e.g.
nxstyle.cfg)? So we don't pollute the source code.


On Sun, Mar 15, 2020 at 7:47 PM Schock, Johannes - NIVUS GmbH
 wrote:
>
> Hello,
> I filed a pull request that introduces an idea of controlling the
> behaviour of nxstyle from inside the file under test.
> https://github.com/apache/incubator-nuttx/pull/569
> This would allow to address some special requirements that are needed in
> cornercases (e.g. whitelisting), and, by using the ignore feature, to
> switch of testing for ranges of the file that are correct but nxstyle
> isn't happy about it (yet unaddressed cornercases).
> This would give a contributor/commiter the possibility to explicitly pass
> the PR through the style check, and perhaps file an nxstyle issue.
> From time to time we could grep for the ignore command and remove it if
> the issue had been resolved.
> Please share your thoughts.
> Johannes
>
> P.S.: if some aren't happy with the xml-like syntax, we can change this
> for sure

Re: Should we relax precheck a little bit?

[Xiang Xiao]

How about we put this special treatment to another file(e.g.
nxstyle.cfg)? So we don't pollute the source code.


On Sun, Mar 15, 2020 at 7:47 PM Schock, Johannes - NIVUS GmbH
 wrote:
>
> Hello,
> I filed a pull request that introduces an idea of controlling the behaviour 
> of nxstyle from inside the file under test.
> https://github.com/apache/incubator-nuttx/pull/569
> This would allow to address some special requirements that are needed in 
> cornercases (e.g. whitelisting), and, by using the ignore feature, to switch 
> of testing for ranges of the file that are correct but nxstyle isn't happy 
> about it (yet unaddressed cornercases).
> This would give a contributor/commiter the possibility to explicitly pass the 
> PR through the style check, and perhaps file an nxstyle issue.
> From time to time we could grep for the ignore command and remove it if the 
> issue had been resolved.
> Please share your thoughts.
> Johannes
>
> P.S.: if some aren't happy with the xml-like syntax, we can change this for 
> sure

Re: Should we relax precheck a little bit?

[spudaneco]

I am oppose to modyfing source files to support the tool.  I don't think we 
should go that direction.Sent from Samsung tablet.
 Original message From: "Schock, Johannes - NIVUS GmbH" 
 Date: 3/15/20  5:47 AM  (GMT-06:00) To: 
dev@nuttx.apache.org Subject: Re: Should we relax precheck a little bit? 
Hello,I filed a pull request that introduces an idea of controlling the 
behaviour of nxstyle from inside the file under 
test.https://github.com/apache/incubator-nuttx/pull/569This would allow to 
address some special requirements that are needed in cornercases (e.g. 
whitelisting), and, by using the ignore feature, to switch of testing for 
ranges of the file that are correct but nxstyle isn't happy about it (yet 
unaddressed cornercases).This would give a contributor/commiter the possibility 
to explicitly pass the PR through the style check, and perhaps file an nxstyle 
issue.From time to time we could grep for the ignore command and remove it if 
the issue had been resolved.Please share your thoughts.JohannesP.S.: if some 
aren't happy with the xml-like syntax, we can change this for sure

Re: Should we relax precheck a little bit?

[Schock, Johannes - NIVUS GmbH]

Hello,
I filed a pull request that introduces an idea of controlling the behaviour of 
nxstyle from inside the file under test.
https://github.com/apache/incubator-nuttx/pull/569
This would allow to address some special requirements that are needed in 
cornercases (e.g. whitelisting), and, by using the ignore feature, to switch of 
testing for ranges of the file that are correct but nxstyle isn't happy about 
it (yet unaddressed cornercases).
This would give a contributor/commiter the possibility to explicitly pass the 
PR through the style check, and perhaps file an nxstyle issue.
>From time to time we could grep for the ignore command and remove it if the 
>issue had been resolved.
Please share your thoughts.
Johannes

P.S.: if some aren't happy with the xml-like syntax, we can change this for sure

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

 true
> MacroBlockBegin: ''
> MacroBlockEnd:   ''
> MaxEmptyLinesToKeep: 1
> NamespaceIndentation: None
> ObjCBinPackProtocolList: Auto
> ObjCBlockIndentWidth: 2
> ObjCSpaceAfterProperty: false
> ObjCSpaceBeforeProtocolList: true
> PenaltyBreakAssignment: 2
> PenaltyBreakBeforeFirstCallParameter: 19
> PenaltyBreakComment: 300
> PenaltyBreakFirstLessLess: 120
> PenaltyBreakString: 1000
> PenaltyBreakTemplateDeclaration: 10
> PenaltyExcessCharacter: 100
> PenaltyReturnTypeOnItsOwnLine: 60
> PointerAlignment: Right
> ReflowComments:  false
> SortIncludes:false
> SortUsingDeclarations: true
> SpaceAfterCStyleCast: false
> SpaceAfterLogicalNot: false
> SpaceAfterTemplateKeyword: true
> SpaceBeforeAssignmentOperators: true
> SpaceBeforeCpp11BracedList: false
> SpaceBeforeCtorInitializerColon: true
> SpaceBeforeInheritanceColon: true
> SpaceBeforeParens: ControlStatements
> SpaceBeforeRangeBasedForLoopColon: true
> SpaceInEmptyParentheses: false
> SpacesBeforeTrailingComments: 1
> SpacesInAngles:  false
> SpacesInContainerLiterals: true
> SpacesInCStyleCastParentheses: false
> SpacesInParentheses: false
> SpacesInSquareBrackets: false
> Standard:Cpp03
> StatementMacros:
>   - Q_UNUSED
>   - QT_REQUIRE_VERSION
> TabWidth:8
> UseTab:  Never
> ...
>
>
>
>
> On Sat, Mar 14, 2020 at 1:29 AM David Sidrane 
> wrote:
>
>> Hi Adam and Maciej,
>>
>> Thank you for spending you time on this. It will be a huge win for
>> everyone!
>>
>> The way I have been looking at this is like in transfer-function with an
>> offset. Once we can get a tools that will take X and make into X" that has
>> well known malformations. We just fix the malformations. So once you feel
>> have something that is close, let's evaluate it and see what the "last
>> mile"
>> looks like.
>>
>> David
>>
>> -Original Message-
>> From: Adam Feuer [mailto:a...@starcat.io]
>> Sent: Friday, March 13, 2020 7:08 PM
>> To: dev@nuttx.apache.org
>> Subject: Re: Should we relax precheck a little bit?
>>
>> Maciej,
>>
>> Thank you! I didn't know about the IndentPPDirectives option! I will try
>> it! :)
>>
>> -adam
>>
>> On Fri, Mar 13, 2020 at 5:16 PM Maciej Wójcik  wrote:
>>
>> > Are you sure that clang-format cannot indent macros? What about
>> >
>> >   IndentPPDirectives: PPDIS_AfterHash
>> >
>> > It also treats the outmost macro in headers in a special way.
>> >
>> > On Sat, 14 Mar 2020, 01:03 Adam Feuer,  wrote:
>> >
>> > > David,
>> > >
>> > > Re: whatstyle, I ran it overnight on the c files in sched/ and came up
>> > with
>> > > a clang-format that does somewhat ok. Thanks for pointing that program
>> > out.
>> > >
>> > > By looking at the output of the diff, I learned a lot about how hard
>> it
>> > is
>> > > to manually format programs. :)
>> > >
>> > > Anyway, the biggest problem with clang-format seems to be the way it
>> > > handles C-macros. In NuttX, they are often indented like this:
>> > >
>> > > #ifdef ...
>> > > #  define ...
>> > > #  ifdef ...
>> > > #define
>> > > #  endif
>> > > #endif
>> > >
>> > > Peter Van Der Perk also mentioned this. There's no stock way to make
>> > > clang-format do that. Maybe a post-processing script that only looked
>> at
>> > > these macros would work. Or a contribution to clang-format. I'll think
>> > > about it some more.
>> > >
>> > > -adam
>> > >
>> > > On Sun, Mar 8, 2020 at 3:40 AM David Sidrane > >
>> > > wrote:
>> > >
>> > > > Hi Adam,
>> > > >
>> > > > Have a look at https://github.com/mikr/whatstyle
>> > > >
>> > > > I got furthest with clang-format and it. It may be we get a 95% of
>> the
>> > > way
>> > > > there with it and we can add a backend secondary scripts.
>> > > >
>> > > > I was unable to convince Greg to create a master template so my
>> > approach
>> > > > was
>> > > > to combine all the files and run it on the set so it would get all
>> the
>> > > > constructs at once.
>> > > >
>> > > > David
>> > > >
>> > >
>> > > --
>> > > Adam Feuer 
>> > >
>> >
>>
>>
>> --
>> Adam Feuer 
>>
>
>
> --
> Adam Feuer 
>


-- 
Adam Feuer 

Re: Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[Adam Feuer]

InCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard:Cpp03
StatementMacros:
  - Q_UNUSED
  - QT_REQUIRE_VERSION
TabWidth:8
UseTab:  Never
...




On Sat, Mar 14, 2020 at 1:29 AM David Sidrane 
wrote:

> Hi Adam and Maciej,
>
> Thank you for spending you time on this. It will be a huge win for
> everyone!
>
> The way I have been looking at this is like in transfer-function with an
> offset. Once we can get a tools that will take X and make into X" that has
> well known malformations. We just fix the malformations. So once you feel
> have something that is close, let's evaluate it and see what the "last
> mile"
> looks like.
>
> David
>
> -Original Message-
> From: Adam Feuer [mailto:a...@starcat.io]
> Sent: Friday, March 13, 2020 7:08 PM
> To: dev@nuttx.apache.org
> Subject: Re: Should we relax precheck a little bit?
>
> Maciej,
>
> Thank you! I didn't know about the IndentPPDirectives option! I will try
> it! :)
>
> -adam
>
> On Fri, Mar 13, 2020 at 5:16 PM Maciej Wójcik  wrote:
>
> > Are you sure that clang-format cannot indent macros? What about
> >
> >   IndentPPDirectives: PPDIS_AfterHash
> >
> > It also treats the outmost macro in headers in a special way.
> >
> > On Sat, 14 Mar 2020, 01:03 Adam Feuer,  wrote:
> >
> > > David,
> > >
> > > Re: whatstyle, I ran it overnight on the c files in sched/ and came up
> > with
> > > a clang-format that does somewhat ok. Thanks for pointing that program
> > out.
> > >
> > > By looking at the output of the diff, I learned a lot about how hard it
> > is
> > > to manually format programs. :)
> > >
> > > Anyway, the biggest problem with clang-format seems to be the way it
> > > handles C-macros. In NuttX, they are often indented like this:
> > >
> > > #ifdef ...
> > > #  define ...
> > > #  ifdef ...
> > > #define
> > > #  endif
> > > #endif
> > >
> > > Peter Van Der Perk also mentioned this. There's no stock way to make
> > > clang-format do that. Maybe a post-processing script that only looked
> at
> > > these macros would work. Or a contribution to clang-format. I'll think
> > > about it some more.
> > >
> > > -adam
> > >
> > > On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
> > > wrote:
> > >
> > > > Hi Adam,
> > > >
> > > > Have a look at https://github.com/mikr/whatstyle
> > > >
> > > > I got furthest with clang-format and it. It may be we get a 95% of
> the
> > > way
> > > > there with it and we can add a backend secondary scripts.
> > > >
> > > > I was unable to convince Greg to create a master template so my
> > approach
> > > > was
> > > > to combine all the files and run it on the set so it would get all
> the
> > > > constructs at once.
> > > >
> > > > David
> > > >
> > >
> > > --
> > > Adam Feuer 
> > >
> >
>
>
> --
> Adam Feuer 
>


-- 
Adam Feuer 

Nuttx Code Formatter Progress [Was RE: Should we relax precheck a little bit?]

[David Sidrane]

Hi Adam and Maciej,

Thank you for spending you time on this. It will be a huge win for everyone!

The way I have been looking at this is like in transfer-function with an
offset. Once we can get a tools that will take X and make into X" that has
well known malformations. We just fix the malformations. So once you feel
have something that is close, let's evaluate it and see what the "last mile"
looks like.

David

-Original Message-
From: Adam Feuer [mailto:a...@starcat.io]
Sent: Friday, March 13, 2020 7:08 PM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

Maciej,

Thank you! I didn't know about the IndentPPDirectives option! I will try
it! :)

-adam

On Fri, Mar 13, 2020 at 5:16 PM Maciej Wójcik  wrote:

> Are you sure that clang-format cannot indent macros? What about
>
>   IndentPPDirectives: PPDIS_AfterHash
>
> It also treats the outmost macro in headers in a special way.
>
> On Sat, 14 Mar 2020, 01:03 Adam Feuer,  wrote:
>
> > David,
> >
> > Re: whatstyle, I ran it overnight on the c files in sched/ and came up
> with
> > a clang-format that does somewhat ok. Thanks for pointing that program
> out.
> >
> > By looking at the output of the diff, I learned a lot about how hard it
> is
> > to manually format programs. :)
> >
> > Anyway, the biggest problem with clang-format seems to be the way it
> > handles C-macros. In NuttX, they are often indented like this:
> >
> > #ifdef ...
> > #  define ...
> > #  ifdef ...
> > #define
> > #  endif
> > #endif
> >
> > Peter Van Der Perk also mentioned this. There's no stock way to make
> > clang-format do that. Maybe a post-processing script that only looked at
> > these macros would work. Or a contribution to clang-format. I'll think
> > about it some more.
> >
> > -adam
> >
> > On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
> > wrote:
> >
> > > Hi Adam,
> > >
> > > Have a look at https://github.com/mikr/whatstyle
> > >
> > > I got furthest with clang-format and it. It may be we get a 95% of the
> > way
> > > there with it and we can add a backend secondary scripts.
> > >
> > > I was unable to convince Greg to create a master template so my
> approach
> > > was
> > > to combine all the files and run it on the set so it would get all the
> > > constructs at once.
> > >
> > > David
> > >
> >
> > --
> > Adam Feuer 
> >
>


-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Adam Feuer]

Maciej,

Thank you! I didn't know about the IndentPPDirectives option! I will try
it! :)

-adam

On Fri, Mar 13, 2020 at 5:16 PM Maciej Wójcik  wrote:

> Are you sure that clang-format cannot indent macros? What about
>
>   IndentPPDirectives: PPDIS_AfterHash
>
> It also treats the outmost macro in headers in a special way.
>
> On Sat, 14 Mar 2020, 01:03 Adam Feuer,  wrote:
>
> > David,
> >
> > Re: whatstyle, I ran it overnight on the c files in sched/ and came up
> with
> > a clang-format that does somewhat ok. Thanks for pointing that program
> out.
> >
> > By looking at the output of the diff, I learned a lot about how hard it
> is
> > to manually format programs. :)
> >
> > Anyway, the biggest problem with clang-format seems to be the way it
> > handles C-macros. In NuttX, they are often indented like this:
> >
> > #ifdef ...
> > #  define ...
> > #  ifdef ...
> > #define
> > #  endif
> > #endif
> >
> > Peter Van Der Perk also mentioned this. There's no stock way to make
> > clang-format do that. Maybe a post-processing script that only looked at
> > these macros would work. Or a contribution to clang-format. I'll think
> > about it some more.
> >
> > -adam
> >
> > On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
> > wrote:
> >
> > > Hi Adam,
> > >
> > > Have a look at https://github.com/mikr/whatstyle
> > >
> > > I got furthest with clang-format and it. It may be we get a 95% of the
> > way
> > > there with it and we can add a backend secondary scripts.
> > >
> > > I was unable to convince Greg to create a master template so my
> approach
> > > was
> > > to combine all the files and run it on the set so it would get all the
> > > constructs at once.
> > >
> > > David
> > >
> >
> > --
> > Adam Feuer 
> >
>


-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Maciej Wójcik]

Are you sure that clang-format cannot indent macros? What about

  IndentPPDirectives: PPDIS_AfterHash

It also treats the outmost macro in headers in a special way.

On Sat, 14 Mar 2020, 01:03 Adam Feuer,  wrote:

> David,
>
> Re: whatstyle, I ran it overnight on the c files in sched/ and came up with
> a clang-format that does somewhat ok. Thanks for pointing that program out.
>
> By looking at the output of the diff, I learned a lot about how hard it is
> to manually format programs. :)
>
> Anyway, the biggest problem with clang-format seems to be the way it
> handles C-macros. In NuttX, they are often indented like this:
>
> #ifdef ...
> #  define ...
> #  ifdef ...
> #define
> #  endif
> #endif
>
> Peter Van Der Perk also mentioned this. There's no stock way to make
> clang-format do that. Maybe a post-processing script that only looked at
> these macros would work. Or a contribution to clang-format. I'll think
> about it some more.
>
> -adam
>
> On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
> wrote:
>
> > Hi Adam,
> >
> > Have a look at https://github.com/mikr/whatstyle
> >
> > I got furthest with clang-format and it. It may be we get a 95% of the
> way
> > there with it and we can add a backend secondary scripts.
> >
> > I was unable to convince Greg to create a master template so my approach
> > was
> > to combine all the files and run it on the set so it would get all the
> > constructs at once.
> >
> > David
> >
>
> --
> Adam Feuer 
>

Re: Should we relax precheck a little bit?

[Adam Feuer]

David,

Re: whatstyle, I ran it overnight on the c files in sched/ and came up with
a clang-format that does somewhat ok. Thanks for pointing that program out.

By looking at the output of the diff, I learned a lot about how hard it is
to manually format programs. :)

Anyway, the biggest problem with clang-format seems to be the way it
handles C-macros. In NuttX, they are often indented like this:

#ifdef ...
#  define ...
#  ifdef ...
#define
#  endif
#endif

Peter Van Der Perk also mentioned this. There's no stock way to make
clang-format do that. Maybe a post-processing script that only looked at
these macros would work. Or a contribution to clang-format. I'll think
about it some more.

-adam

On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
wrote:

> Hi Adam,
>
> Have a look at https://github.com/mikr/whatstyle
>
> I got furthest with clang-format and it. It may be we get a 95% of the way
> there with it and we can add a backend secondary scripts.
>
> I was unable to convince Greg to create a master template so my approach
> was
> to combine all the files and run it on the set so it would get all the
> constructs at once.
>
> David
>

-- 
Adam Feuer 

RE: Should we relax precheck a little bit?

[David Sidrane]

See inline

-Original Message-
From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
Sent: Wednesday, March 11, 2020 8:11 AM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

It isn't good to change the licenses in one patch because:
1.We need review all licenses change more carefully, mixing style
change make all reviewer lose the focus.
[DBS] +1
2.People outside the team may need recheck the licenses change to
enuse no any IP pollution.
3.We need find a way to revert the licenses change easily if we do
something wrong.
[DBS] +1 separate commit per file, 1 or N PRs - NOT Squashed on merge.

Finally, it's bad practice to make the unrelated change in one patch,
just like you do all operation in a huge function.
The best approach is:
1.Change licenses in one patch
[DBS] Change licenses 1 commit.
2.Fix the style issue in another patch
[DBS] Change style 1 commit.
3.Bug fix or new function in self contained patch

[DBS] We also should have a tool to test for binary identity of the builds.
This way all the documentation ONLY changes can be verified to be
documentation ONLY changes.

On Wed, Mar 11, 2020 at 8:59 PM Nathan Hartman 
wrote:
>
> On Wed, Mar 11, 2020 at 8:13 AM David Sidrane 
> wrote:
>
> > First, I have to say that I value the NuttX coding standard  - it make
> > for
> > the most readable code I have EVERY worked on. I want it preserved.
> >
> > But in light of the below discussion: I am changing my position that the
> > "rule" CI is to just check ONLY the changes to a file AND if the only CI
> > failure is a minor style failure the committer may commit it.
> >
> > To be clear I am not saying that a foreign camel cased contribution is
> > allowed in. Just small stuff.
> >
> > Rational:
> >
> > Way back, on the list, and in email it was decided that Job #1 was
> > having a
> > valid style check tool.
> > For some it was a foregone conclusion that if CI did style checking the
> > tool
> > had to 100% accurate and provide go no go results.
> >
> > If we allocated more time fixing the tool, than reinventing ways to run
> > CI,
> > we would not be in the state we are in. The message here is: We should
> > do a
> > better job prioritizing and avoiding reinventing the wheel.
> >
> > Why avoid the root cause. We need to fix the tool.
> >
> > Now from a what is required to move forward point of view: We have been
> > living under a belief that all the code in the OS 100% conformed to the
> > coding standard, and expected that the violations were mostly in arch.
> > None
> > of this has proven true. The reality of what this means is, that we will
> > have to touch every file in the system, (this was expected, but just for
> > licenses changes). Running a constantly changing, and sometimes broken
> > tool
> > constantly on every file in the system is a waste of resources. So we
> > need
> > to fix the tool _THEN_ fix the code with it.
>
>
> I suggest, until that happens, to do two things in parallel:
>
> (1) make it the interim policy that *changed* lines have to conform. Over
> time this will reduce nonconforming code.
>
> (2) during the process of fixing licenses, fix also the coding style of
> affected files.
>
> In other words, if changing code, changed lines must conform; if changing
> license, make whole file conform.
>
> Rationale: Commits that make functional change should be limited to just
> that functional change and nothing else, for purposes of review, revert,
> etc. (License change is not functional change.)
>
> Cheers
> Nathan

Re: Should we relax precheck a little bit?

[Xiang Xiao]

It isn't good to change the licenses in one patch because:
1.We need review all licenses change more carefully, mixing style
change make all reviewer lose the focus.
2.People outside the team may need recheck the licenses change to
enuse no any IP pollution.
3.We need find a way to revert the licenses change easily if we do
something wrong.
Finally, it's bad practice to make the unrelated change in one patch,
just like you do all operation in a huge function.
The best approach is:
1.Change licenses in one patch
2.Fix the style issue in another patch
3.Bug fix or new function in self contained patch

On Wed, Mar 11, 2020 at 8:59 PM Nathan Hartman  wrote:
>
> On Wed, Mar 11, 2020 at 8:13 AM David Sidrane 
> wrote:
>
> > First, I have to say that I value the NuttX coding standard  - it make for
> > the most readable code I have EVERY worked on. I want it preserved.
> >
> > But in light of the below discussion: I am changing my position that the
> > "rule" CI is to just check ONLY the changes to a file AND if the only CI
> > failure is a minor style failure the committer may commit it.
> >
> > To be clear I am not saying that a foreign camel cased contribution is
> > allowed in. Just small stuff.
> >
> > Rational:
> >
> > Way back, on the list, and in email it was decided that Job #1 was having a
> > valid style check tool.
> > For some it was a foregone conclusion that if CI did style checking the
> > tool
> > had to 100% accurate and provide go no go results.
> >
> > If we allocated more time fixing the tool, than reinventing ways to run CI,
> > we would not be in the state we are in. The message here is: We should do a
> > better job prioritizing and avoiding reinventing the wheel.
> >
> > Why avoid the root cause. We need to fix the tool.
> >
> > Now from a what is required to move forward point of view: We have been
> > living under a belief that all the code in the OS 100% conformed to the
> > coding standard, and expected that the violations were mostly in arch. None
> > of this has proven true. The reality of what this means is, that we will
> > have to touch every file in the system, (this was expected, but just for
> > licenses changes). Running a constantly changing, and sometimes broken tool
> > constantly on every file in the system is a waste of resources. So we need
> > to fix the tool _THEN_ fix the code with it.
>
>
> I suggest, until that happens, to do two things in parallel:
>
> (1) make it the interim policy that *changed* lines have to conform. Over
> time this will reduce nonconforming code.
>
> (2) during the process of fixing licenses, fix also the coding style of
> affected files.
>
> In other words, if changing code, changed lines must conform; if changing
> license, make whole file conform.
>
> Rationale: Commits that make functional change should be limited to just
> that functional change and nothing else, for purposes of review, revert,
> etc. (License change is not functional change.)
>
> Cheers
> Nathan

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Wed, Mar 11, 2020 at 5:43 PM 张铎(Duo Zhang)  wrote:
>
> 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.
>

Duo, this is the 2nd option I listed before, checkpatch.sh already
support to just check the modified lines with -r.

> Xiang Xiao  于2020年3月11日周三 下午3:42写道：
>
> > On Wed, Mar 11, 2020 at 2:49 PM Takashi Yamamoto
> >  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 
> > 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
> >

Re: Should we relax precheck a little bit?

[Nathan Hartman]

On Wed, Mar 11, 2020 at 8:13 AM David Sidrane 
wrote:

> First, I have to say that I value the NuttX coding standard  - it make for
> the most readable code I have EVERY worked on. I want it preserved.
>
> But in light of the below discussion: I am changing my position that the
> "rule" CI is to just check ONLY the changes to a file AND if the only CI
> failure is a minor style failure the committer may commit it.
>
> To be clear I am not saying that a foreign camel cased contribution is
> allowed in. Just small stuff.
>
> Rational:
>
> Way back, on the list, and in email it was decided that Job #1 was having a
> valid style check tool.
> For some it was a foregone conclusion that if CI did style checking the
> tool
> had to 100% accurate and provide go no go results.
>
> If we allocated more time fixing the tool, than reinventing ways to run CI,
> we would not be in the state we are in. The message here is: We should do a
> better job prioritizing and avoiding reinventing the wheel.
>
> Why avoid the root cause. We need to fix the tool.
>
> Now from a what is required to move forward point of view: We have been
> living under a belief that all the code in the OS 100% conformed to the
> coding standard, and expected that the violations were mostly in arch. None
> of this has proven true. The reality of what this means is, that we will
> have to touch every file in the system, (this was expected, but just for
> licenses changes). Running a constantly changing, and sometimes broken tool
> constantly on every file in the system is a waste of resources. So we need
> to fix the tool _THEN_ fix the code with it.


I suggest, until that happens, to do two things in parallel:

(1) make it the interim policy that *changed* lines have to conform. Over
time this will reduce nonconforming code.

(2) during the process of fixing licenses, fix also the coding style of
affected files.

In other words, if changing code, changed lines must conform; if changing
license, make whole file conform.

Rationale: Commits that make functional change should be limited to just
that functional change and nothing else, for purposes of review, revert,
etc. (License change is not functional change.)

Cheers
Nathan

RE: Should we relax precheck a little bit?

[David Sidrane]

First, I have to say that I value the NuttX coding standard  - it make for
the most readable code I have EVERY worked on. I want it preserved.

But in light of the below discussion: I am changing my position that the
"rule" CI is to just check ONLY the changes to a file AND if the only CI
failure is a minor style failure the committer may commit it.

To be clear I am not saying that a foreign camel cased contribution is
allowed in. Just small stuff.

Rational:

Way back, on the list, and in email it was decided that Job #1 was having a
valid style check tool.
For some it was a foregone conclusion that if CI did style checking the tool
had to 100% accurate and provide go no go results.

If we allocated more time fixing the tool, than reinventing ways to run CI,
we would not be in the state we are in. The message here is: We should do a
better job prioritizing and avoiding reinventing the wheel.

Why avoid the root cause. We need to fix the tool.

Now from a what is required to move forward point of view: We have been
living under a belief that all the code in the OS 100% conformed to the
coding standard, and expected that the violations were mostly in arch. None
of this has proven true. The reality of what this means is, that we will
have to touch every file in the system, (this was expected, but just for
licenses changes). Running a constantly changing, and sometimes broken tool
constantly on every file in the system is a waste of resources. So we need
to fix the tool _THEN_ fix the code with it.


David

-Original Message-

From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
Sent: Wednesday, March 11, 2020 12:42 AM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

On Wed, Mar 11, 2020 at 2:49 PM Takashi Yamamoto
 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 
> 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

Re: Should we relax precheck a little bit?

[Duo Zhang]

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  于2020年3月11日周三 下午3:42写道：

> On Wed, Mar 11, 2020 at 2:49 PM Takashi Yamamoto
>  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 
> 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
>

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Wed, Mar 11, 2020 at 2:49 PM Takashi Yamamoto
 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  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

Re: Should we relax precheck a little bit?

[Takashi Yamamoto]

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.

On Sun, Mar 8, 2020 at 3:11 AM Xiang Xiao  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

Re: Should we relax precheck a little bit?

[Takashi Yamamoto]

On Tue, Mar 10, 2020 at 3:43 PM Xiang Xiao  wrote:
>
> On Tue, Mar 10, 2020 at 1:17 AM Gregory Nutt  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 ] [-v ] [-r ] 
> >  nxstyle -h this help
> >  nxstyle -v  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.
>
> >
> >

Re: Re: Should we relax precheck a little bit?

[Takashi Yamamoto]

On Mon, Mar 9, 2020 at 6:09 AM Peter Van Der Perk
 wrote:
>
> Hi Adam,
>
> I've been trying to make a clang-format for NuttX however I did run in some 
> nasty bugs with clang-format.
> Mostly the inconsistent spacing caused by preprocessor directives (#ifdef) 
> etc.
> I've filed a bug on the LLVM bugtracker 
> (https://bugs.llvm.org/show_bug.cgi?id=44843) unfortunely I didn't get any 
> response.
> Please find below the clang-format I've used, I would say it's 80% done.
> But it's mostly that NXStyle requires special things that clang-format 
> doesn’t provide.

can you give me an example of special things?

>
> ---
> AccessModifierOffset: -6
> AlignAfterOpenBracket: Align
> AlignConsecutiveAssignments: 'true'
> AlignConsecutiveDeclarations: 'false'
> AlignEscapedNewlines: DontAlign
> AlignTrailingComments: 'true'
> AllowShortFunctionsOnASingleLine: None
> AllowShortIfStatementsOnASingleLine: 'false'
> AllowShortLoopsOnASingleLine: 'false'
> AlwaysBreakAfterDefinitionReturnType: None
> AlwaysBreakAfterReturnType: AllDefinitions
> BreakBeforeBinaryOperators: None
> BraceWrapping:
>   AfterCaseLabel:  true
>   AfterClass:  true
>   AfterControlStatement: true
>   AfterEnum:   true
>   AfterFunction:   true
>   AfterNamespace:  true
>   AfterObjCDeclaration: true
>   AfterStruct: true
>   AfterUnion:  true
>   BeforeCatch: true
>   BeforeElse:  true
>   IndentBraces:true
>   SplitEmptyFunction: true
>   SplitEmptyRecord: true
>   SplitEmptyNamespace: true
> BreakBeforeBraces: Custom
> ColumnLimit: 77
> CommentPragmas: '^ IWYU pragma:'
> ConstructorInitializerIndentWidth: '0'
> ContinuationIndentWidth: 2
> IncludeIsMainRegex: (Test)?$
> IndentPPDirectives: AfterHash
> IndentWidth: '2'
> JavaScriptQuotes: Leave
> MaxEmptyLinesToKeep: 2
> NamespaceIndentation: None
> ObjCBlockIndentWidth: '3'
> PenaltyBreakBeforeFirstCallParameter: '32'
> PenaltyBreakComment: 0
> PenaltyBreakFirstLessLess: '44'
> PenaltyBreakString: 838
> PenaltyExcessCharacter: '399916'
> PenaltyReturnTypeOnItsOwnLine: 30
> PointerAlignment: Right
> ReflowComments: 'true'
> SortIncludes: 'false'
> SpaceAfterCStyleCast: 'false'
> SpaceBeforeAssignmentOperators: 'true'
> SpaceBeforeParens: ControlStatements
> SpaceInEmptyParentheses: 'false'
> SpacesBeforeTrailingComments: 1
> SpacesInCStyleCastParentheses: 'false'
> SpacesInParentheses: 'false'
> SpacesInSquareBrackets: 'false'
> Standard: Cpp11
> TabWidth: '4'
> UseTab: Never
> ...
>
> > Thanks David. I'll try your approach. If there are some things that don't 
> > quite work with Clang-Format (I already found a few) I'll see about adding 
> > a fixup script pass at the end, or contributing some rules back to Clang.
> >
> > I'll try your idea about combining all the files under sched into a set.
> >
> > When you said you got 95% of the way there, do you have a .clang-format 
> > file that I could use as a starting point? If so that would help me start 
> > where you left off.
> >
> >cheers
> >adam

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Tue, Mar 10, 2020 at 1:17 AM Gregory Nutt  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.

>
> > - 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 ] [-v ] [-r ] 
>  nxstyle -h this help
>  nxstyle -v  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.

>
>

Re: Should we relax precheck a little bit?

[Adam Feuer]

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

Re: Should we relax precheck a little bit?

[Gregory Nutt]

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.

Re: Should we relax precheck a little bit?

[Gregory Nutt]

How much time should we wait before starting a vote?

When you know the vote will be in favour of your position is the time to call a 
vote. Actually in favour is probably too strong a position to take, it’s more 
that no-one would be against this course of action even if they may 100% agree 
with it. People may not agree, but collectively everyone thinks this is the 
best way forward. Getting consensus can sometimes be difficult and involve a 
lot of discussion.
We don't have an established voting procedure as do other projects, but 
most of us have been using a 72-hour [DISCUSS] thread preceding any 
72-hour [VOTE].  Certainly, there must be some focused dialogue before 
having any vote.  Embedding opinions and pronouncements in emails along 
with other non-focused discussion will not be be effective.

RE: Should we relax precheck a little bit?

[David Sidrane]

I agree with Alin.
-1

-Original Message-
From: Alin Jerpelea [mailto:jerpe...@gmail.com]
Sent: Monday, March 09, 2020 5:36 AM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

-1
I thin that as a long term goal we should aim to have it enabled and leave
each contributor to fix the styles as commits come in
This way we will gradually have all violations fixed

Alin

On Mon, Mar 9, 2020 at 1:22 PM Justin Mclean 
wrote:

> Hi,
>
> Sorry late here - what I should have said is “may NOT 100% agree with it.”.
>
> Justin

Re: Should we relax precheck a little bit?

[Alin Jerpelea]

-1
I thin that as a long term goal we should aim to have it enabled and leave
each contributor to fix the styles as commits come in
This way we will gradually have all violations fixed

Alin

On Mon, Mar 9, 2020 at 1:22 PM Justin Mclean 
wrote:

> Hi,
>
> Sorry late here - what I should have said is “may NOT 100% agree with it.”.
>
> Justin

Re: Should we relax precheck a little bit?

[Justin Mclean]

Hi,

Sorry late here - what I should have said is “may NOT 100% agree with it.”.

Justin

Re: Should we relax precheck a little bit?

[Justin Mclean]

Hi,

> How much time should we wait before starting a vote?

When you know the vote will be in favour of your position is the time to call a 
vote. Actually in favour is probably too strong a position to take, it’s more 
that no-one would be against this course of action even if they may 100% agree 
with it. People may not agree, but collectively everyone thinks this is the 
best way forward. Getting consensus can sometimes be difficult and involve a 
lot of discussion.

Thanks,
Justin 

Re: Should we relax precheck a little bit?

[Takashi Yamamoto]

On Sun, Mar 8, 2020 at 9:30 AM Gregory Nutt  wrote:
>
>
> > Since there's no current maintainer for nxstyle... What would people think
> > about trying Clang-Format ?
> >
> > It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
> > Microsoft ), and
> > can be configured for many different style guides... it should be possible
> > to configure it for NuttX's style guide. Or at least get close.
> >
> > If there's interest, I can take a shot at trying to configure it using the
> > NuttX style guide. If we went that direction, we'd have another tool to
> > install. But then we'd only have to maintain a configuration, and we'd be
> > joining a big community who are all using this same tool.
> >
> > What do you think?
>
> We have already tried indent, astyle, and uncrustify and were not happy
> with them.  There are starts already in tools/.  Of those, indent
> probably works the same.
>
> A tool that produces absolutely perfect output would be useful. But even
> the tiniest of bugs make the tool useless.
>
> My guess is that you will fail.  Everyone else has.  So cannot in good
> faith encourage you.
>
> But if you create the perfect tool that does everything 100% correctly,
> I suppose you would be a hero.  The acceptance test is this:
>
> You run the program against all .c and .h files under sched and the
> output is 100% compatible with the input you win.  One byte different
> you lose.  Hundreds hours have gone into this challenge and all have failed.

assuming that a tool has --try-run/--check-only,
i suppose it doesn't need to be that perfect to replace nxstyle.

>
> Greg

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Mon, Mar 9, 2020 at 12:52 PM Justin Mclean  wrote:
>
> Hi,
>
> > I basically agree with most of the things you say here, but in an Apache 
> > project, you cannot set rules unilaterally.  No one person has that 
> > authority over the project.  We all all equals.  Decisions can only be made 
> > with concurrence from other team members and we can only get concurrence 
> > through a full vote.
>
> Refining that a bit. It true no one has authority but it's best to try and to 
> reach consensus by discussion. Voting too often or too early makes winners 
> and losers and splits the community. Vote when done is usually made to 
> confirm that consensus and when called teh outcome is already known.
>

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.
Here is the several reasons:
1.It's hard to review the real change if we mix the style change in
PR, here is two examples:
   https://github.com/apache/incubator-nuttx/pull/492
   https://github.com/apache/incubator-nuttx/pull/495
   The change in these two PR is simple:
   1.Fix the warning: implicit declaration of function 'uart_earlyserialinit'
   2.Change the argument and return of spi_send from uint16_t to uint32_t
   But it is very hard to review now because more than 90% change is
to fix the style issue.
2.The github action still ensure the modified lines confirm the coding
standard, so the relaxtion don't make the situation worse.
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.
>

How much time should we wait before starting a vote?

> Thanks,
> Justin

Re: Should we relax precheck a little bit?

[Justin Mclean]

Hi,

> I basically agree with most of the things you say here, but in an Apache 
> project, you cannot set rules unilaterally.  No one person has that authority 
> over the project.  We all all equals.  Decisions can only be made with 
> concurrence from other team members and we can only get concurrence through a 
> full vote.

Refining that a bit. It true no one has authority but it's best to try and to 
reach consensus by discussion. Voting too often or too early makes winners and 
losers and splits the community. Vote when done is usually made to confirm that 
consensus and when called teh outcome is already known.

I would not suggest having a vote on this in such a short time frame.

Thanks,
Justin

Re: Should we relax precheck a little bit?

[Adam Feuer]

Pre- or post- processor I meant, depending on what would work best.

-adam

On Sun, Mar 8, 2020 at 2:29 PM Adam Feuer  wrote:

> Thanks Greg. I haven't tried indent, I will try it with the config you
> suggest, if it can get close I'll try a pre-processor script with it too.
>
> -adam
>
> On Sun, Mar 8, 2020 at 2:21 PM Gregory Nutt  wrote:
>
>> The best pretty printer for NuttX that I am aware of is still
>> tools/indent.sh.  It consistently screws up a few things (as listed in
>> tools/README.txt).  But the screw-ups are relatively easy and probably
>> could be postprocesses.  The astyle and uncrustify stuff in tools/ is
>> pretty must useles.
>>
>> But none of the tools handle subtle things, like aligned of assignments
>> on = operators like:
>>
>>dog  = bat;   /* Assign dog */
>>pony = lizard;/* Assign pony */
>>aardvark = pangolin;  /* Assign aardvark */
>>
>> and none handle this awful case:
>>
>> #ifdef HAVE_SOMETHINGELSE
>>if (something == somethingelse)
>>  {
>> do(somethingelse);
>>  }
>>else
>> #endif
>>if (something == nothing)
>>  {
>>do(nothing);
>>  }
>>
>> It is not clear what the indentation should be for the if outside of the
>> #ifdef.. #endif.  Most code aligns that if as though the previous
>> conditional logic did not exist.
>>
>> Another issue is the unconditional compound statement which breaks all
>> of indentation rules (but this is an nxstyle problem).
>>
>>{
>>  int i
>> for (i = 0; i < n; i++)
>>{
>>  handle(i);
>>}
>>}
>>
>> The outer {} is indented only by two which makes the indentation of all
>> of the inner stuff appear wrong to nxstyle.
>>
>>
>>
>
> --
> Adam Feuer 
>


-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Adam Feuer]

Thanks Greg. I haven't tried indent, I will try it with the config you
suggest, if it can get close I'll try a pre-processor script with it too.

-adam

On Sun, Mar 8, 2020 at 2:21 PM Gregory Nutt  wrote:

> The best pretty printer for NuttX that I am aware of is still
> tools/indent.sh.  It consistently screws up a few things (as listed in
> tools/README.txt).  But the screw-ups are relatively easy and probably
> could be postprocesses.  The astyle and uncrustify stuff in tools/ is
> pretty must useles.
>
> But none of the tools handle subtle things, like aligned of assignments
> on = operators like:
>
>dog  = bat;   /* Assign dog */
>pony = lizard;/* Assign pony */
>aardvark = pangolin;  /* Assign aardvark */
>
> and none handle this awful case:
>
> #ifdef HAVE_SOMETHINGELSE
>if (something == somethingelse)
>  {
> do(somethingelse);
>  }
>else
> #endif
>if (something == nothing)
>  {
>do(nothing);
>  }
>
> It is not clear what the indentation should be for the if outside of the
> #ifdef.. #endif.  Most code aligns that if as though the previous
> conditional logic did not exist.
>
> Another issue is the unconditional compound statement which breaks all
> of indentation rules (but this is an nxstyle problem).
>
>{
>  int i
> for (i = 0; i < n; i++)
>{
>  handle(i);
>}
>}
>
> The outer {} is indented only by two which makes the indentation of all
> of the inner stuff appear wrong to nxstyle.
>
>
>

-- 
Adam Feuer 

Re: Re: Should we relax precheck a little bit?

[Adam Feuer]

Thanks Peter! I will try out your config. If it can get close, I will see
if I can create a post-processor like David said.

cheers
adam

On Sun, Mar 8, 2020 at 2:09 PM Peter Van Der Perk 
wrote:

> Hi Adam,
>
> I've been trying to make a clang-format for NuttX however I did run in
> some nasty bugs with clang-format.
> Mostly the inconsistent spacing caused by preprocessor directives (#ifdef)
> etc.
> I've filed a bug on the LLVM bugtracker (
> https://bugs.llvm.org/show_bug.cgi?id=44843) unfortunely I didn't get any
> response.
> Please find below the clang-format I've used, I would say it's 80% done.
> But it's mostly that NXStyle requires special things that clang-format
> doesn’t provide.
>
> ---
> AccessModifierOffset: -6
> AlignAfterOpenBracket: Align
> AlignConsecutiveAssignments: 'true'
> AlignConsecutiveDeclarations: 'false'
> AlignEscapedNewlines: DontAlign
> AlignTrailingComments: 'true'
> AllowShortFunctionsOnASingleLine: None
> AllowShortIfStatementsOnASingleLine: 'false'
> AllowShortLoopsOnASingleLine: 'false'
> AlwaysBreakAfterDefinitionReturnType: None
> AlwaysBreakAfterReturnType: AllDefinitions
> BreakBeforeBinaryOperators: None
> BraceWrapping:
>   AfterCaseLabel:  true
>   AfterClass:  true
>   AfterControlStatement: true
>   AfterEnum:   true
>   AfterFunction:   true
>   AfterNamespace:  true
>   AfterObjCDeclaration: true
>   AfterStruct: true
>   AfterUnion:  true
>   BeforeCatch: true
>   BeforeElse:  true
>   IndentBraces:true
>   SplitEmptyFunction: true
>   SplitEmptyRecord: true
>   SplitEmptyNamespace: true
> BreakBeforeBraces: Custom
> ColumnLimit: 77
> CommentPragmas: '^ IWYU pragma:'
> ConstructorInitializerIndentWidth: '0'
> ContinuationIndentWidth: 2
> IncludeIsMainRegex: (Test)?$
> IndentPPDirectives: AfterHash
> IndentWidth: '2'
> JavaScriptQuotes: Leave
> MaxEmptyLinesToKeep: 2
> NamespaceIndentation: None
> ObjCBlockIndentWidth: '3'
> PenaltyBreakBeforeFirstCallParameter: '32'
> PenaltyBreakComment: 0
> PenaltyBreakFirstLessLess: '44'
> PenaltyBreakString: 838
> PenaltyExcessCharacter: '399916'
> PenaltyReturnTypeOnItsOwnLine: 30
> PointerAlignment: Right
> ReflowComments: 'true'
> SortIncludes: 'false'
> SpaceAfterCStyleCast: 'false'
> SpaceBeforeAssignmentOperators: 'true'
> SpaceBeforeParens: ControlStatements
> SpaceInEmptyParentheses: 'false'
> SpacesBeforeTrailingComments: 1
> SpacesInCStyleCastParentheses: 'false'
> SpacesInParentheses: 'false'
> SpacesInSquareBrackets: 'false'
> Standard: Cpp11
> TabWidth: '4'
> UseTab: Never
> ...
>
> > Thanks David. I'll try your approach. If there are some things that
> don't quite work with Clang-Format (I already found a few) I'll see about
> adding a fixup script pass at the end, or contributing some rules back to
> Clang.
> >
> > I'll try your idea about combining all the files under sched into a set.
> >
> > When you said you got 95% of the way there, do you have a .clang-format
> file that I could use as a starting point? If so that would help me start
> where you left off.
> >
> >cheers
> >adam
>


-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Gregory Nutt]

The best pretty printer for NuttX that I am aware of is still 
tools/indent.sh.  It consistently screws up a few things (as listed in 
tools/README.txt).  But the screw-ups are relatively easy and probably 
could be postprocesses.  The astyle and uncrustify stuff in tools/ is 
pretty must useles.


But none of the tools handle subtle things, like aligned of assignments 
on = operators like:


  dog  = bat;   /* Assign dog */
  pony = lizard;    /* Assign pony */
  aardvark = pangolin;  /* Assign aardvark */

and none handle this awful case:

   #ifdef HAVE_SOMETHINGELSE
  if (something == somethingelse)
    {
   do(somethingelse);
    }
  else
   #endif
  if (something == nothing)
    {
  do(nothing);
    }

It is not clear what the indentation should be for the if outside of the 
#ifdef.. #endif.  Most code aligns that if as though the previous 
conditional logic did not exist.


Another issue is the unconditional compound statement which breaks all 
of indentation rules (but this is an nxstyle problem).


  {
    int i
   for (i = 0; i < n; i++)
  {
    handle(i);
  }
  }

The outer {} is indented only by two which makes the indentation of all 
of the inner stuff appear wrong to nxstyle.

RE: Re: Should we relax precheck a little bit?

[Peter Van Der Perk]

Hi Adam,

I've been trying to make a clang-format for NuttX however I did run in some 
nasty bugs with clang-format.
Mostly the inconsistent spacing caused by preprocessor directives (#ifdef) etc.
I've filed a bug on the LLVM bugtracker 
(https://bugs.llvm.org/show_bug.cgi?id=44843) unfortunely I didn't get any 
response.
Please find below the clang-format I've used, I would say it's 80% done.
But it's mostly that NXStyle requires special things that clang-format doesn’t 
provide.

---
AccessModifierOffset: -6
AlignAfterOpenBracket: Align
AlignConsecutiveAssignments: 'true'
AlignConsecutiveDeclarations: 'false'
AlignEscapedNewlines: DontAlign
AlignTrailingComments: 'true'
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: 'false'
AllowShortLoopsOnASingleLine: 'false'
AlwaysBreakAfterDefinitionReturnType: None
AlwaysBreakAfterReturnType: AllDefinitions
BreakBeforeBinaryOperators: None
BraceWrapping:
  AfterCaseLabel:  true
  AfterClass:  true
  AfterControlStatement: true
  AfterEnum:   true
  AfterFunction:   true
  AfterNamespace:  true
  AfterObjCDeclaration: true
  AfterStruct: true
  AfterUnion:  true
  BeforeCatch: true
  BeforeElse:  true
  IndentBraces:true
  SplitEmptyFunction: true
  SplitEmptyRecord: true
  SplitEmptyNamespace: true
BreakBeforeBraces: Custom
ColumnLimit: 77
CommentPragmas: '^ IWYU pragma:'
ConstructorInitializerIndentWidth: '0'
ContinuationIndentWidth: 2
IncludeIsMainRegex: (Test)?$
IndentPPDirectives: AfterHash
IndentWidth: '2'
JavaScriptQuotes: Leave
MaxEmptyLinesToKeep: 2
NamespaceIndentation: None
ObjCBlockIndentWidth: '3'
PenaltyBreakBeforeFirstCallParameter: '32'
PenaltyBreakComment: 0
PenaltyBreakFirstLessLess: '44'
PenaltyBreakString: 838
PenaltyExcessCharacter: '399916'
PenaltyReturnTypeOnItsOwnLine: 30
PointerAlignment: Right
ReflowComments: 'true'
SortIncludes: 'false'
SpaceAfterCStyleCast: 'false'
SpaceBeforeAssignmentOperators: 'true'
SpaceBeforeParens: ControlStatements
SpaceInEmptyParentheses: 'false'
SpacesBeforeTrailingComments: 1
SpacesInCStyleCastParentheses: 'false'
SpacesInParentheses: 'false'
SpacesInSquareBrackets: 'false'
Standard: Cpp11
TabWidth: '4'
UseTab: Never
...

> Thanks David. I'll try your approach. If there are some things that don't 
> quite work with Clang-Format (I already found a few) I'll see about adding a 
> fixup script pass at the end, or contributing some rules back to Clang.
> 
> I'll try your idea about combining all the files under sched into a set.
> 
> When you said you got 95% of the way there, do you have a .clang-format file 
> that I could use as a starting point? If so that would help me start where 
> you left off.
> 
>cheers
>adam

Re: FW: Should we relax precheck a little bit?

[Adam Feuer]

Got it. Thanks.

On Sun, Mar 8, 2020 at 11:10 AM David Sidrane 
wrote:

> > When you said you got 95% of the way there, do you have a .clang-format
> file that I could use as a starting point?
>
> No. I did not get there. I was just referring to to the last % as a script.
>
> -Original Message-
> From: Adam Feuer [mailto:a...@starcat.io]
> Sent: Sunday, March 08, 2020 9:11 AM
> To: dev@nuttx.apache.org
> Subject: Re: Should we relax precheck a little bit?
>
> Thanks David. I'll try your approach. If there are some things that don't
> quite work with Clang-Format (I already found a few) I'll see about adding
> a fixup script pass at the end, or contributing some rules back to Clang.
>
> I'll try your idea about combining all the files under sched into a set.
>
> When you said you got 95% of the way there, do you have a .clang-format
> file that I could use as a starting point? If so that would help me start
> where you left off.
>
> cheers
> adam
>
> On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
> wrote:
>
> > Hi Adam,
> >
> > Have a look at https://github.com/mikr/whatstyle
> >
> > I got furthest with clang-format and it. It may be we get a 95% of the
> way
> > there with it and we can add a backend secondary scripts.
> >
> > I was unable to convince Greg to create a master template so my approach
> > was
> > to combine all the files and run it on the set so it would get all the
> > constructs at once.
> >
> > David
> >
> > -Original Message-
> > From: Adam Feuer [mailto:a...@starcat.io]
> > Sent: Saturday, March 07, 2020 4:01 PM
> > To: dev@nuttx.apache.org
> > Subject: Re: Should we relax precheck a little bit?
> >
> > Since there's no current maintainer for nxstyle... What would people
> think
> > about trying Clang-Format <http://clang.llvm.org/docs/ClangFormat.html>?
> >
> > It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
> > Microsoft <http://clang.llvm.org/docs/ClangFormatStyleOptions.html>),
> and
> > can be configured for many different style guides... it should be
> possible
> > to configure it for NuttX's style guide. Or at least get close.
> >
> > If there's interest, I can take a shot at trying to configure it using
> the
> > NuttX style guide. If we went that direction, we'd have another tool to
> > install. But then we'd only have to maintain a configuration, and we'd be
> > joining a big community who are all using this same tool.
> >
> > What do you think?
> >
> > -adam
> >
> > On Sat, Mar 7, 2020 at 3:24 PM Gregory Nutt  wrote:
> >
> > >
> > > > +1 for fixing nxstyle (or configuring another tool like Clang Format
> > > > <http://clang.llvm.org/docs/ClangFormat.html>)
> > > >
> > > > That would make it a lot easier to submit PRs that are in the right
> > > format,
> > > > at least :)
> > >
> > > There is no one dedicated to maintaining nxstyle right now.  I wrote
> the
> > > original*, but there was once a plan for Haitao Liu to take that over.
> > > Others (YAMT) and been making good contributions.  So I would not
> expect
> > > any snappy response to nxstyle problems right now.
> > >
> > > Greg
> > >
> > > * Just a little CYA and history.  nsstyle starting out as a tiny hack
> > > tool that I used to check a few things in files.  It got re-used a few
> > > times and grew and now it is a key program in the workflow.  It is
> > > unfortunate fact that the tool is woefully under designed.  Provided
> > > that we set up some good validation tests, I really think it should be
> > > redesigned to at least carefully reviewed to make sure that it can
> > > actually support the things that we expect from it.
> > >
> > > It is a dumb tool, it knows nothing about C syntax.  I don't think it
> > > really needs fully understand C syntax, but it does need to understand
> > > the context of things better.  Currently it is just a collection of
> > > heuristics that spots a few landmarks and makes interfaces from
> > > comparison of some patterns.  So it is not very capable.
> > >
> > >
> > >
> > >
> >
> > --
> > Adam Feuer 
> >
>
>
> --
> Adam Feuer 
>


-- 
Adam Feuer 

FW: Should we relax precheck a little bit?

[David Sidrane]

> When you said you got 95% of the way there, do you have a .clang-format
file that I could use as a starting point?

No. I did not get there. I was just referring to to the last % as a script.

-Original Message-
From: Adam Feuer [mailto:a...@starcat.io]
Sent: Sunday, March 08, 2020 9:11 AM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

Thanks David. I'll try your approach. If there are some things that don't
quite work with Clang-Format (I already found a few) I'll see about adding
a fixup script pass at the end, or contributing some rules back to Clang.

I'll try your idea about combining all the files under sched into a set.

When you said you got 95% of the way there, do you have a .clang-format
file that I could use as a starting point? If so that would help me start
where you left off.

cheers
adam

On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
wrote:

> Hi Adam,
>
> Have a look at https://github.com/mikr/whatstyle
>
> I got furthest with clang-format and it. It may be we get a 95% of the way
> there with it and we can add a backend secondary scripts.
>
> I was unable to convince Greg to create a master template so my approach
> was
> to combine all the files and run it on the set so it would get all the
> constructs at once.
>
> David
>
> -Original Message-
> From: Adam Feuer [mailto:a...@starcat.io]
> Sent: Saturday, March 07, 2020 4:01 PM
> To: dev@nuttx.apache.org
> Subject: Re: Should we relax precheck a little bit?
>
> Since there's no current maintainer for nxstyle... What would people think
> about trying Clang-Format <http://clang.llvm.org/docs/ClangFormat.html>?
>
> It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
> Microsoft <http://clang.llvm.org/docs/ClangFormatStyleOptions.html>), and
> can be configured for many different style guides... it should be possible
> to configure it for NuttX's style guide. Or at least get close.
>
> If there's interest, I can take a shot at trying to configure it using the
> NuttX style guide. If we went that direction, we'd have another tool to
> install. But then we'd only have to maintain a configuration, and we'd be
> joining a big community who are all using this same tool.
>
> What do you think?
>
> -adam
>
> On Sat, Mar 7, 2020 at 3:24 PM Gregory Nutt  wrote:
>
> >
> > > +1 for fixing nxstyle (or configuring another tool like Clang Format
> > > <http://clang.llvm.org/docs/ClangFormat.html>)
> > >
> > > That would make it a lot easier to submit PRs that are in the right
> > format,
> > > at least :)
> >
> > There is no one dedicated to maintaining nxstyle right now.  I wrote the
> > original*, but there was once a plan for Haitao Liu to take that over.
> > Others (YAMT) and been making good contributions.  So I would not expect
> > any snappy response to nxstyle problems right now.
> >
> > Greg
> >
> > * Just a little CYA and history.  nsstyle starting out as a tiny hack
> > tool that I used to check a few things in files.  It got re-used a few
> > times and grew and now it is a key program in the workflow.  It is
> > unfortunate fact that the tool is woefully under designed.  Provided
> > that we set up some good validation tests, I really think it should be
> > redesigned to at least carefully reviewed to make sure that it can
> > actually support the things that we expect from it.
> >
> > It is a dumb tool, it knows nothing about C syntax.  I don't think it
> > really needs fully understand C syntax, but it does need to understand
> > the context of things better.  Currently it is just a collection of
> > heuristics that spots a few landmarks and makes interfaces from
> > comparison of some patterns.  So it is not very capable.
> >
> >
> >
> >
>
> --
> Adam Feuer 
>


-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Adam Feuer]

Thanks David. I'll try your approach. If there are some things that don't
quite work with Clang-Format (I already found a few) I'll see about adding
a fixup script pass at the end, or contributing some rules back to Clang.

I'll try your idea about combining all the files under sched into a set.

When you said you got 95% of the way there, do you have a .clang-format
file that I could use as a starting point? If so that would help me start
where you left off.

cheers
adam

On Sun, Mar 8, 2020 at 3:40 AM David Sidrane 
wrote:

> Hi Adam,
>
> Have a look at https://github.com/mikr/whatstyle
>
> I got furthest with clang-format and it. It may be we get a 95% of the way
> there with it and we can add a backend secondary scripts.
>
> I was unable to convince Greg to create a master template so my approach
> was
> to combine all the files and run it on the set so it would get all the
> constructs at once.
>
> David
>
> -Original Message-
> From: Adam Feuer [mailto:a...@starcat.io]
> Sent: Saturday, March 07, 2020 4:01 PM
> To: dev@nuttx.apache.org
> Subject: Re: Should we relax precheck a little bit?
>
> Since there's no current maintainer for nxstyle... What would people think
> about trying Clang-Format <http://clang.llvm.org/docs/ClangFormat.html>?
>
> It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
> Microsoft <http://clang.llvm.org/docs/ClangFormatStyleOptions.html>), and
> can be configured for many different style guides... it should be possible
> to configure it for NuttX's style guide. Or at least get close.
>
> If there's interest, I can take a shot at trying to configure it using the
> NuttX style guide. If we went that direction, we'd have another tool to
> install. But then we'd only have to maintain a configuration, and we'd be
> joining a big community who are all using this same tool.
>
> What do you think?
>
> -adam
>
> On Sat, Mar 7, 2020 at 3:24 PM Gregory Nutt  wrote:
>
> >
> > > +1 for fixing nxstyle (or configuring another tool like Clang Format
> > > <http://clang.llvm.org/docs/ClangFormat.html>)
> > >
> > > That would make it a lot easier to submit PRs that are in the right
> > format,
> > > at least :)
> >
> > There is no one dedicated to maintaining nxstyle right now.  I wrote the
> > original*, but there was once a plan for Haitao Liu to take that over.
> > Others (YAMT) and been making good contributions.  So I would not expect
> > any snappy response to nxstyle problems right now.
> >
> > Greg
> >
> > * Just a little CYA and history.  nsstyle starting out as a tiny hack
> > tool that I used to check a few things in files.  It got re-used a few
> > times and grew and now it is a key program in the workflow.  It is
> > unfortunate fact that the tool is woefully under designed.  Provided
> > that we set up some good validation tests, I really think it should be
> > redesigned to at least carefully reviewed to make sure that it can
> > actually support the things that we expect from it.
> >
> > It is a dumb tool, it knows nothing about C syntax.  I don't think it
> > really needs fully understand C syntax, but it does need to understand
> > the context of things better.  Currently it is just a collection of
> > heuristics that spots a few landmarks and makes interfaces from
> > comparison of some patterns.  So it is not very capable.
> >
> >
> >
> >
>
> --
> Adam Feuer 
>


-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Gregory Nutt]

Hi, Xiang,

I basically agree with most of the things you say here, but in an Apache 
project, you cannot set rules unilaterally.  No one person has that 
authority over the project.  We all all equals.  Decisions can only be 
made with concurrence from other team members and we can only get 
concurrence through a full vote.


I think you should:

1. Update the workflow document at 
https://cwiki.apache.org/confluence/display/NUTTX/Code+Contribution+Workflow+--+Brennan+Ashton 
so that it provides  complete descriptions of the step from where some 
decides to modify code until that change is incorporated into the 
repository.


2. Call for a 72-hour [DISCUSS] phase to review the workflow, then then

3. A [VOTE] to approve the workflow.

No direction from one person to another is appropriate without that 
concurrence.  Even then, nothing is really binding.  No one can be 
punished for not following the rules other than through peer pressure.


I appreciate that you might probably be uncomfortable about updating the 
workflow document in a language that is not your native language.  But 
we can help you through that through the review process.


Greg

Re: Should we relax precheck a little bit?

[Justin Mclean]

Hi,

> But how we measure the release become better and better?

Very simply you succeeded if you attract more users and committers. Being 
welcoming to new committers and having a simple process which that are able to 
take part helps that. I think you are looking though this from a technical lens 
which is perhaps not always the right way to do so.

> Both are the reasonable indicator that the project is on the right direction, 
> is it right?

Sadly I’ve seen projects that take this approach and wither and die, but that 
not to say there is only one path. Try to think community over code.

Thanks,
Justin

RE: Should we relax precheck a little bit?

[David Sidrane]

Hi Adam,

Have a look at https://github.com/mikr/whatstyle

I got furthest with clang-format and it. It may be we get a 95% of the way
there with it and we can add a backend secondary scripts.

I was unable to convince Greg to create a master template so my approach was
to combine all the files and run it on the set so it would get all the
constructs at once.

David

-Original Message-
From: Adam Feuer [mailto:a...@starcat.io]
Sent: Saturday, March 07, 2020 4:01 PM
To: dev@nuttx.apache.org
Subject: Re: Should we relax precheck a little bit?

Since there's no current maintainer for nxstyle... What would people think
about trying Clang-Format <http://clang.llvm.org/docs/ClangFormat.html>?

It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
Microsoft <http://clang.llvm.org/docs/ClangFormatStyleOptions.html>), and
can be configured for many different style guides... it should be possible
to configure it for NuttX's style guide. Or at least get close.

If there's interest, I can take a shot at trying to configure it using the
NuttX style guide. If we went that direction, we'd have another tool to
install. But then we'd only have to maintain a configuration, and we'd be
joining a big community who are all using this same tool.

What do you think?

-adam

On Sat, Mar 7, 2020 at 3:24 PM Gregory Nutt  wrote:

>
> > +1 for fixing nxstyle (or configuring another tool like Clang Format
> > <http://clang.llvm.org/docs/ClangFormat.html>)
> >
> > That would make it a lot easier to submit PRs that are in the right
> format,
> > at least :)
>
> There is no one dedicated to maintaining nxstyle right now.  I wrote the
> original*, but there was once a plan for Haitao Liu to take that over.
> Others (YAMT) and been making good contributions.  So I would not expect
> any snappy response to nxstyle problems right now.
>
> Greg
>
> * Just a little CYA and history.  nsstyle starting out as a tiny hack
> tool that I used to check a few things in files.  It got re-used a few
> times and grew and now it is a key program in the workflow.  It is
> unfortunate fact that the tool is woefully under designed.  Provided
> that we set up some good validation tests, I really think it should be
> redesigned to at least carefully reviewed to make sure that it can
> actually support the things that we expect from it.
>
> It is a dumb tool, it knows nothing about C syntax.  I don't think it
> really needs fully understand C syntax, but it does need to understand
> the context of things better.  Currently it is just a collection of
> heuristics that spots a few landmarks and makes interfaces from
> comparison of some patterns.  So it is not very capable.
>
>
>
>

-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[David Sidrane]

Xiang,

Ok. see your point. It is valid and true the new changes do not add to the 
problem.

I just do not agree with the way of contributing. I look at it as if I touch a 
file, I take ownership and pride in making it better. We would not be being 
having this discussion if we had a tool that works and can format the codebase. 
 But we are where we are. So why don't you call a vote?



On 2020/03/08 04:13:11, Xiang Xiao  wrote: 
> On Sun, Mar 8, 2020 at 2:46 AM David Sidrane  wrote:
> >
> > -1 It is Not inline with long term goal and a violation of the Inviolable
> >
> 
> David,
> No new violation here, the code modified in patch still must pass the
> coding style check, the tool just ignored the unmodified part.
> 
> > o Expediency is not a justification for violating the coding standard.
> >
> > -Original Message-
> > From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
> > Sent: Saturday, March 07, 2020 10:11 AM
> > To: dev@nuttx.apache.org
> > Subject: Should we relax precheck a little bit?
> >
> > 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
> 

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Sun, Mar 8, 2020 at 4:52 PM Justin Mclean
 wrote:
>
> Hi,
>
> > Yes, this is the key point why I am asking this question: we need
> > stable the mainline and make the first apache release.
>
> Apache doesn’t actually care if your release works or not [*], it not an ASF  
> requirement that a build must pass all tests. What matters is that the next 
> build is better than the last one. We would prefer that project make frequent 
> releases, gradually improving things, and not put off a release trying to 
> make it perfect. One path leads to community growth, the other to community 
> decline, I’ll let you guess which is which.

But how we measure the release become better and better?
1.All config/host build without error
2.The testsuit has the growing pass rate
Both are the reasonable indicator that the project is on the right
direction, is it right?


>
> Thanks,
> Justin
>
> [*] The project and it’s user might care more about this :-)

Re: Should we relax precheck a little bit?

[Justin Mclean]

Hi,

> Yes, this is the key point why I am asking this question: we need
> stable the mainline and make the first apache release. 

Apache doesn’t actually care if your release works or not [*], it not an ASF  
requirement that a build must pass all tests. What matters is that the next 
build is better than the last one. We would prefer that project make frequent 
releases, gradually improving things, and not put off a release trying to make 
it perfect. One path leads to community growth, the other to community decline, 
I’ll let you guess which is which.

Thanks,
Justin

[*] The project and it’s user might care more about this :-)

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Sun, Mar 8, 2020 at 2:50 AM Alan Carvalho de Assis  wrote:
>
> Hi Xiang,
>
> Normally Greg, Abdelatif, I and others are fixing these nxstyles
> issues when someone submit a patch. It is described in the legacy
> process of merging PR:
> https://acassis.wordpress.com/2020/01/02/the-old-way-nuttx-workflow/
>

First, I would suggest that all commiters stop to modify the code in
PR branch and then merge into master branch. This process most likely
generate build break once per week.

> I don't know if it is fair or unfair to share/transfer this
> responsibility to people submitting patches. But I recall how it was
> boring to submit patches to Linux kernel, they used to reject my
> patches many times until all issues were fixed.
>

Yes, I also submit the patch to Linux kernel, but kernel maintainer
just request the people fix the issue in his patch, not the problem
already exists in the code base, right?

> The reported errors from nxstyle could be included directly as a
> comment in the PR, then the author could see it and fix it.
>
> If we relax the process I think more styles violations will be
> included on mainline. In the order hand if people start to care about
> it eventually all files will get rid of issues.
>

As I reply to David, we don't relax the changed made by people, all
lines in the patch still go through nxstyle check, what I suggest is
that we ignore the unmodified part. The codebase never become the
worse with this relaxation. The maintainer still can provide the
followup PR to fix the style issue in the rest files to make the
mainline better.

> More important the release a new version ASAP is to guarantee we are
> building a solid base to make the entire process more robust. I think
> we are going in this direction.
>

Yes, this is the key point why I am asking this question: we need
stable the mainline and make the first apache release. But how we can
get the stable mainline? The minimum requirement is that all
config/host pass the build.
The precheck is online on now which is only method to guarantee that
each new patch don't break the build, but most commiters just ignore
the error reported by precheck and merge the PR to mainline, here is a
partial list in yesterday:
https://github.com/apache/incubator-nuttx/pull/471/checks
https://github.com/apache/incubator-nuttx/pull/479/checks
https://github.com/apache/incubator-nuttx/pull/471/checks
...
Do you think we can get the buildable mainline with this process if we
consider NuttX contain more than 1000 option and nobody can try all
possible config/host locally?
Actually, I don't care who fix the style issue in the original code
base(contributor or committer), the more important is that all PR must
pass the precheck and nobody modify the PR locally before merging..
Since the precheck is ready now, I would suggest that we modify the
current workflow to better utilize the precheck:
1.Don't create a local PR branch, make some modification and merge the
local PR branch directly to master. This process totally bypass the
precheck and may make the mainline unbuildable.
2.Don't merge the PR before precheck report the pass, I would prefer
we disable merge button during the checking/building.
3.Consider the current codebase isn't style clean and nxstyle isn't in
the perfect state, I suggest nxstyle just check the modified part.
Item 3 is just a suggestion, but please don't modify/merge PR when the
precheck report error,  otherwise we can never get a stable mainline
ready for release.

> Just my 2 cents.
>
> BR,
>
> Alan
>
> On 3/7/20, Xiang Xiao  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
> >

Re: Should we relax precheck a little bit?

[Xiang Xiao]

On Sun, Mar 8, 2020 at 2:46 AM David Sidrane  wrote:
>
> -1 It is Not inline with long term goal and a violation of the Inviolable
>

David,
No new violation here, the code modified in patch still must pass the
coding style check, the tool just ignored the unmodified part.

> o Expediency is not a justification for violating the coding standard.
>
> -Original Message-
> From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
> Sent: Saturday, March 07, 2020 10:11 AM
> To: dev@nuttx.apache.org
> Subject: Should we relax precheck a little bit?
>
> 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

Re: Should we relax precheck a little bit?

[Adam Feuer]

On Sat, Mar 7, 2020 at 4:30 PM Gregory Nutt  wrote:

> The acceptance test is this:
>
> You run the program against all .c and .h files under sched and the
> output is 100% compatible with the input you win.  One byte different
> you lose.  Hundreds hours have gone into this challenge and all have
> failed.
>

Cool. If I try it and succeed I will let you know. :)

cheers
adam
-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Gregory Nutt]

Since there's no current maintainer for nxstyle... What would people think
about trying Clang-Format ?

It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
Microsoft ), and
can be configured for many different style guides... it should be possible
to configure it for NuttX's style guide. Or at least get close.

If there's interest, I can take a shot at trying to configure it using the
NuttX style guide. If we went that direction, we'd have another tool to
install. But then we'd only have to maintain a configuration, and we'd be
joining a big community who are all using this same tool.

What do you think?


We have already tried indent, astyle, and uncrustify and were not happy 
with them.  There are starts already in tools/.  Of those, indent 
probably works the same.


A tool that produces absolutely perfect output would be useful. But even 
the tiniest of bugs make the tool useless.


My guess is that you will fail.  Everyone else has.  So cannot in good 
faith encourage you.


But if you create the perfect tool that does everything 100% correctly, 
I suppose you would be a hero.  The acceptance test is this:


You run the program against all .c and .h files under sched and the 
output is 100% compatible with the input you win.  One byte different 
you lose.  Hundreds hours have gone into this challenge and all have failed.


Greg

Re: Should we relax precheck a little bit?

[Adam Feuer]

Since there's no current maintainer for nxstyle... What would people think
about trying Clang-Format ?

It's a well-used tool (LLVM, Google, Chromium, Mozilla, Webkit, and
Microsoft ), and
can be configured for many different style guides... it should be possible
to configure it for NuttX's style guide. Or at least get close.

If there's interest, I can take a shot at trying to configure it using the
NuttX style guide. If we went that direction, we'd have another tool to
install. But then we'd only have to maintain a configuration, and we'd be
joining a big community who are all using this same tool.

What do you think?

-adam

On Sat, Mar 7, 2020 at 3:24 PM Gregory Nutt  wrote:

>
> > +1 for fixing nxstyle (or configuring another tool like Clang Format
> > )
> >
> > That would make it a lot easier to submit PRs that are in the right
> format,
> > at least :)
>
> There is no one dedicated to maintaining nxstyle right now.  I wrote the
> original*, but there was once a plan for Haitao Liu to take that over.
> Others (YAMT) and been making good contributions.  So I would not expect
> any snappy response to nxstyle problems right now.
>
> Greg
>
> * Just a little CYA and history.  nsstyle starting out as a tiny hack
> tool that I used to check a few things in files.  It got re-used a few
> times and grew and now it is a key program in the workflow.  It is
> unfortunate fact that the tool is woefully under designed.  Provided
> that we set up some good validation tests, I really think it should be
> redesigned to at least carefully reviewed to make sure that it can
> actually support the things that we expect from it.
>
> It is a dumb tool, it knows nothing about C syntax.  I don't think it
> really needs fully understand C syntax, but it does need to understand
> the context of things better.  Currently it is just a collection of
> heuristics that spots a few landmarks and makes interfaces from
> comparison of some patterns.  So it is not very capable.
>
>
>
>

-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Gregory Nutt]

+1 for fixing nxstyle (or configuring another tool like Clang Format
)

That would make it a lot easier to submit PRs that are in the right format,
at least :)


There is no one dedicated to maintaining nxstyle right now.  I wrote the 
original*, but there was once a plan for Haitao Liu to take that over.  
Others (YAMT) and been making good contributions.  So I would not expect 
any snappy response to nxstyle problems right now.


Greg

* Just a little CYA and history.  nsstyle starting out as a tiny hack 
tool that I used to check a few things in files.  It got re-used a few 
times and grew and now it is a key program in the workflow.  It is 
unfortunate fact that the tool is woefully under designed.  Provided 
that we set up some good validation tests, I really think it should be 
redesigned to at least carefully reviewed to make sure that it can 
actually support the things that we expect from it.


It is a dumb tool, it knows nothing about C syntax.  I don't think it 
really needs fully understand C syntax, but it does need to understand 
the context of things better.  Currently it is just a collection of 
heuristics that spots a few landmarks and makes interfaces from 
comparison of some patterns.  So it is not very capable.

Re: Should we relax precheck a little bit?

[Adam Feuer]

+1 for fixing nxstyle (or configuring another tool like Clang Format
)

That would make it a lot easier to submit PRs that are in the right format,
at least :)

-adam

On Sat, Mar 7, 2020 at 11:44 AM Gregory Nutt  wrote:

> But, on the other hand, in the future, wei will want the full checks
> enabled.  So it seems to me the options are:
>
> - Disable full checking temporarily and focus on eliminating the
> problems, or
> - Let things continue as they are.. accept a little pain and get things
> in shape.
>
> To me, there are no surprises here.  I am rather masochistically
> inclined to let things continue as the are but I appreciate the other
> positions as well.
>
> I always try to apply good engineering judgement to the warnings/errors
> from nxstyle and for a personal tool, it is served me we.   But we are
> not going to be able to expect a generic contributor to understand what
> is a quirk in the tool and what is something that needs to be fixed.  A
> first good step would be to make sure that all nstyle issues are
> properly documented in the github issuess.
>
> Greg
>
> On 3/7/2020 12:10 PM, Xiang Xiao 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
>
>
>

-- 
Adam Feuer 

Re: Should we relax precheck a little bit?

[Gregory Nutt]

But, on the other hand, in the future, wei will want the full checks 
enabled.  So it seems to me the options are:


- Disable full checking temporarily and focus on eliminating the 
problems, or
- Let things continue as they are.. accept a little pain and get things 
in shape.


To me, there are no surprises here.  I am rather masochistically 
inclined to let things continue as the are but I appreciate the other 
positions as well.


I always try to apply good engineering judgement to the warnings/errors 
from nxstyle and for a personal tool, it is served me we.   But we are 
not going to be able to expect a generic contributor to understand what 
is a quirk in the tool and what is something that needs to be fixed.  A 
first good step would be to make sure that all nstyle issues are 
properly documented in the github issuess.


Greg

On 3/7/2020 12:10 PM, Xiang Xiao 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

Re: Should we relax precheck a little bit?

[Alan Carvalho de Assis]

Hi Xiang,

Normally Greg, Abdelatif, I and others are fixing these nxstyles
issues when someone submit a patch. It is described in the legacy
process of merging PR:
https://acassis.wordpress.com/2020/01/02/the-old-way-nuttx-workflow/

I don't know if it is fair or unfair to share/transfer this
responsibility to people submitting patches. But I recall how it was
boring to submit patches to Linux kernel, they used to reject my
patches many times until all issues were fixed.

The reported errors from nxstyle could be included directly as a
comment in the PR, then the author could see it and fix it.

If we relax the process I think more styles violations will be
included on mainline. In the order hand if people start to care about
it eventually all files will get rid of issues.

More important the release a new version ASAP is to guarantee we are
building a solid base to make the entire process more robust. I think
we are going in this direction.

Just my 2 cents.

BR,

Alan

On 3/7/20, Xiang Xiao  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
>

RE: Should we relax precheck a little bit?

[David Sidrane]

-1 It is Not inline with long term goal and a violation of the Inviolable

o Expediency is not a justification for violating the coding standard.

-Original Message-
From: Xiang Xiao [mailto:xiaoxiang781...@gmail.com]
Sent: Saturday, March 07, 2020 10:11 AM
To: dev@nuttx.apache.org
Subject: Should we relax precheck a little bit?

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

Should we relax precheck a little bit?

[Xiang Xiao]

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