Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

[Wiles, Keith]

On 1/3/16, 2:06 AM, "Matthew Hall" <mhall@mhcomputing.net> wrote:

>Hello,
>
>In over a month I didn't get a reply if it would be OK to clean up the 
>inconsistent tabs and spaces indents in the pktgen, to make it easier 
>for the community to read the code and help maintain it and add bug fixes.
>
>It would be very helpful if I could get a response and a plan for this 
>if my proposal is not acceptable, or it's not going to be very pleasant 
>to patch any bugs I find when testing it, as some of the stuff I'd want 
>to work on in the pktgen comes from files with weird indentation that is 
>not consistent and thus painful to send patches.

Sorry, Matthew, You sent your email around Thanksgiving/release time and I missed it. If you have questions or comments directly for Pktgen please put [PKTGEN] in the subject line I have a filter for that one.
>
>Sincerely,
>Matthew.
>
>On 11/22/15 11:10 PM, Matthew Hall wrote:
>> I would like to reindent it using the following astyle command, with a few
>> small hand edits past that level, to get it closer to most other DPDK code as
>> the inconsistent mix of tabs, spaces, etc. makes it difficult to read and
>> debug when it has issues.
>>
>> Obviously the upstream Lua and common/wr_* code would not be included as they
>> seem to be copied from elsewhere w/ different upstream standards.
>>
>> If I were to make the big diff needed for this, would this item be acceptable
>> upstream?
>>
>> Otherwise it is going to be hard to get more people working on the code
>> reliably as it will be hard for others to follow besides the original authors.
>>
>> astyle \
>> --max-code-length=132 \
>> --style=attach \
>> --break-closing-brackets \
>> --add-brackets \
>> --indent=force-tab=8 \

Pktgen is setup for tabs for 4 (with replace tabs with spaces), using tab stop of 8 is just wrong IMO :-)
Just started using kdevelop instead of eclipse, so I may have corrupted the style some :-(

At least it is suppose to be done that way. I will reformat the code (with tabs=4) and have a look at the output.
The rules from other places like to replace tabs with spaces, which is OK I guess.

I can run the astyle on the code and look at the output, if it looks OK I will submit it to the repo, then we just need to understand your hand edits.



------------------------
We need to discuss the DPDK coding style not in this email, but as a standalone email on the dev list. But here are my thoughts for now.

A bigger question is what is the coding style of DPDK and where is it documented? I looked in the docs/web site and did not find any coding style suggestions. Maybe I missed it someplace.

Also using something like astyle or uncrustify tool would be a good thing to be done before releases or a patch is submitted.

I know checkpatch likes to check for some items like extra white space characters at end of lines. I like tools that do just one job like astyle or uncrustify or lint tools. IMO checkpatch should not be doing style checks per say, but a couple of minor checks is fine. If we want the code to run through a style checker then lets start using one.


++Keith
>> --indent-switches \
>> --indent-labels \
>> --indent-col1-comments \
>> --pad-oper \
>> --pad-header \
>> --unpad-paren \
>> --align-pointer=type \
>> --align-reference=type \
>> --suffix=none \
>> --lineend=linux \
>> ./app/**/*.{c,h}
>>
>> Sincerely,
>> Matthew.
>>
>


Regards,
Keith

Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

[Matthew Hall]

On 1/3/16 9:09 AM, Wiles, Keith wrote:
> Pktgen is setup for tabs for 4 (with replace tabs with spaces), using tab stop of 8 is just wrong IMO :-)
> Just started using kdevelop instead of eclipse, so I may have corrupted the style some :-(

The problem I found was a number of files had an incompatible 
combination of the two formats.

Personally, I agree tab size 4 w/ spaces instead of tabs is easiest to 
read and edit. But I could live with any space based system for the most 
part. I find tab based systems are unpleasant because it is difficult 
when tabs are used for one thing and spaces for another thing. This 
annoyance also applies to DPDK and the kernel but it's too late for both 
of those.

> At least it is suppose to be done that way. I will reformat the code (with tabs=4) and have a look at the output.

Thanks this will be a big help.

> I can run the astyle on the code and look at the output, if it looks OK I will submit it to the repo

Sounds great... it is no big hurry on my end but I want to start with a 
clean slate before I get invested in the code, and start really hitting 
it hard, and making patches.

The formatting command I provided is not perfect, but it was the best I 
could do with the various popular indenter tools to try to avoid messing 
up too much of the rest of the good code in the files in the process of 
fixing the format.

You might be able to improve it a bit further w/ some additional 
experimentation since you are the original maintainer of the code 
obviously. Or perhaps reformat using tools in Eclipse or KDevelop? I had 
good luck w/ Eclipse before with special configuration but I only mostly 
used the Java mode not the C / C++ one which is less good.

Matthew.

Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

[Stephen Hemminger]

On Sun, 3 Jan 2016 14:07:35 -0800
Matthew Hall <mhall@mhcomputing.net> wrote:

> On 1/3/16 9:09 AM, Wiles, Keith wrote:
> > Pktgen is setup for tabs for 4 (with replace tabs with spaces), using tab stop of 8 is just wrong IMO :-)
> > Just started using kdevelop instead of eclipse, so I may have corrupted the style some :-(
> 
> The problem I found was a number of files had an incompatible 
> combination of the two formats.
> 
> Personally, I agree tab size 4 w/ spaces instead of tabs is easiest to 
> read and edit. But I could live with any space based system for the most 
> part. I find tab based systems are unpleasant because it is difficult 
> when tabs are used for one thing and spaces for another thing. This 
> annoyance also applies to DPDK and the kernel but it's too late for both 
> of those.
> 
> > At least it is suppose to be done that way. I will reformat the code (with tabs=4) and have a look at the output.
> 
> Thanks this will be a big help.
> 
> > I can run the astyle on the code and look at the output, if it looks OK I will submit it to the repo
> 
> Sounds great... it is no big hurry on my end but I want to start with a 
> clean slate before I get invested in the code, and start really hitting 
> it hard, and making patches.
> 
> The formatting command I provided is not perfect, but it was the best I 
> could do with the various popular indenter tools to try to avoid messing 
> up too much of the rest of the good code in the files in the process of 
> fixing the format.
> 
> You might be able to improve it a bit further w/ some additional 
> experimentation since you are the original maintainer of the code 
> obviously. Or perhaps reformat using tools in Eclipse or KDevelop? I had 
> good luck w/ Eclipse before with special configuration but I only mostly 
> used the Java mode not the C / C++ one which is less good.
> 
> Matthew.

Since DPDK mostly follows kernel style, why should this program be different?

Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

[Ferruh Yigit]

On Sun, Jan 03, 2016 at 05:09:16PM +0000, Wiles, Keith wrote:

<snip>

> A bigger question is what is the coding style of DPDK and where is it documented? I looked in the docs/web site and did not find any coding style suggestions. Maybe I missed it someplace.

There is one in:
http://dpdk.org/doc/guides/contributing/coding_style.html

Regards,
ferruh

Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

[Wiles, Keith]

On 1/3/16, 5:35 PM, "Yigit, Ferruh" <ferruh.yigit@intel.com> wrote:

>On Sun, Jan 03, 2016 at 05:09:16PM +0000, Wiles, Keith wrote:
>
><snip>
>
>> A bigger question is what is the coding style of DPDK and where is it documented? I looked in the docs/web site and did not find any coding style suggestions. Maybe I missed it someplace.
>
>There is one in:
>http://dpdk.org/doc/guides/contributing/coding_style.html

Thanks not sure how I missed that. I use the search tool in the html code for coding, style, … I guess I was looking for Coding Style to pop up, it was under contributor’s guidelines instead. :-(

I am playing with uncrustify huge number of option with universalIndentGUI to see if I can get close to our guidelines.

I will post it when I get close.

>
>Regards,
>ferruh
>


Regards,
Keith

Re: [dpdk-dev] [PKTGEN] OK to reindent the pktgen (mix of tabs and spaces, etc.)?

[Wiles, Keith]

On 1/3/16, 4:07 PM, "dev on behalf of Matthew Hall" <dev-bounces@dpdk.org on behalf of mhall@mhcomputing.net> wrote:

>On 1/3/16 9:09 AM, Wiles, Keith wrote:
>> Pktgen is setup for tabs for 4 (with replace tabs with spaces), using tab stop of 8 is just wrong IMO :-)
>> Just started using kdevelop instead of eclipse, so I may have corrupted the style some :-(
>
>The problem I found was a number of files had an incompatible 
>combination of the two formats.
>
>Personally, I agree tab size 4 w/ spaces instead of tabs is easiest to 
>read and edit. But I could live with any space based system for the most 
>part. I find tab based systems are unpleasant because it is difficult 
>when tabs are used for one thing and spaces for another thing. This 
>annoyance also applies to DPDK and the kernel but it's too late for both 
>of those.
>
>> At least it is suppose to be done that way. I will reformat the code (with tabs=4) and have a look at the output.
>
>Thanks this will be a big help.
>
>> I can run the astyle on the code and look at the output, if it looks OK I will submit it to the repo
>
>Sounds great... it is no big hurry on my end but I want to start with a 
>clean slate before I get invested in the code, and start really hitting 
>it hard, and making patches.
>
>The formatting command I provided is not perfect, but it was the best I 
>could do with the various popular indenter tools to try to avoid messing 
>up too much of the rest of the good code in the files in the process of 
>fixing the format.
>
>You might be able to improve it a bit further w/ some additional 
>experimentation since you are the original maintainer of the code 
>obviously. Or perhaps reformat using tools in Eclipse or KDevelop? I had 
>good luck w/ Eclipse before with special configuration but I only mostly 
>used the Java mode not the C / C++ one which is less good.

I push a version of Pktgen with cleaned up formatting using ‘uncrustify 0.61’ and 'UniversalIndentGUI 1.2.0 Rev 1070 13 Aug 2015’. I put the script and configuration file in the top directory. The formatting looks close to DPDK coding guidelines, but I am sure some tweaks would need to be done.

Uncrustify has a huge number of options and not always clear as to the effect an option has on the code. Header files seems to get effected a bit more then C code as the formatting is usual done by hand I guess. The only way to see the deltas is with UnviersalIndentGUI tool.

All of this is on the ‘dev’ branch, so let me know what you think.

++Keith
>
>Matthew.
>


Regards,
Keith