Re: [Xen-devel] [PATCH] Add clang-format file for Xen Hypervisor format

[Lars Kurth <lars.kurth.xen@xxxxxxxxx>]

> On 12 Apr 2017, at 12:51, Ishani <chugh.ishani@xxxxxxxxxxxxxxxxxxx> wrote:
> 
> Hello,
> 
> Thanks for the comments.
> I have created a markdown file for clang-format doc which I will add. 
> 
>> If so, you want to include this information in the cover letter (see above). 
>> Or you >could write little bash script which shows how this works
> 
> Can you elaborate on the task of bash script? Should it give the difference 
> between files?

Please don't top-post (see 
https://en.wikipedia.org/wiki/Posting_style#Top-posting) but use 
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style instead
Top-posting makes it really hard to follow a review on a mailing list. In this 
case I had to go down into the e-mail thread and identify the context of "If 
so, you want to include this information in the cover letter ..."

I added the reply in the right place below.

> Shall I create a new patch by incorporating the suggested changes?

Yes, basically, you want to follow 
https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Review.2C_Rinse_.26_Repeat

> Regards,
> Ishani
> 
> ----- Original Message -----
> From: "Lars Kurth" <lars.kurth.xen@xxxxxxxxx>
> To: "Ishani Chugh" <chugh.ishani@xxxxxxxxxxxxxxxxxxx>
> Cc: "xen-devel" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "cardoe" <cardoe@xxxxxxxxxx>
> Sent: Wednesday, April 12, 2017 4:56:53 PM
> Subject: Re: [PATCH] Add clang-format file for Xen Hypervisor format
> 
> Hi Ishani,
> 
>> On 12 Apr 2017, at 11:49, Ishani Chugh <chugh.ishani@xxxxxxxxxxxxxxxxxxx> 
>> wrote:
> 
> Here you would normally add a short description of the patch (also called 
> cover letter). In this case, you probably want to describe:
> a) What you have done
> b) Link to 
> https://wiki.xenproject.org/wiki/Outreach_Program_Projects#Code_Standards_Checking_using_clang-format,
>  
> https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit
>  and 
> https://docs.google.com/document/d/10NJn-QvO1TvyJJJGE2PD6FtElYCT3neBAffIqeWHdiE/edit
> c) You can also link to your github repo
> d) You may want to add the two command lines or some instructions showing how 
> to use the patch
> 
>> Signed-off-by: Ishani Chugh <chugh.ishani@xxxxxxxxxxxxxxxxxxx>
>> ---
>> tools/clang-format/tests/.clang-format | 88 
>> ++++++++++++++++++++++++++++++++++
>> tools/clang-format/tests/correct.c     | 73 ++++++++++++++++++++++++++++
>> tools/clang-format/tests/test.c        | 69 ++++++++++++++++++++++++++
>> 3 files changed, 230 insertions(+)
>> create mode 100644 tools/clang-format/tests/.clang-format
>> create mode 100644 tools/clang-format/tests/correct.c
>> create mode 100644 tools/clang-format/tests/test.c
>> 
>> diff --git a/tools/clang-format/tests/.clang-format 
>> b/tools/clang-format/tests/.clang-format
>> new file mode 100644
>> index 0000000..2229910
>> --- /dev/null
>> +++ b/tools/clang-format/tests/.clang-format
>> @@ -0,0 +1,88 @@
>> +---
>> +Language:        Cpp
>> +# BasedOnStyle:  LLVM
> 
> This would be a good place to list in a comment, which coding style patterns 
> the file covers. In other words, everything that has a yes in 
> https://docs.google.com/document/d/1tCcwZ9K38ToLGTPHZkfs2sS4s4YulrGoH8LIHwBMbg4/edit
> 
> The other thing, which is a little unclear is whether the file applies to 
> - http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=CODING_STYLE only
> - http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/CODING_STYLE 
> only
> - or both
> 
> I think it is OK to just cover one file.

...

>> diff --git a/tools/clang-format/tests/test.c 
>> b/tools/clang-format/tests/test.c
>> new file mode 100644
>> index 0000000..2e7eaf2
>> --- /dev/null
>> +++ b/tools/clang-format/tests/test.c
> 
> How is this file different from correct.c? 
> It looks to me that test.c is a wrongly formatted version of correct.c and 
> when you pass it into clang-format it is re-formatted to match correct.c
> 
> If so, you want to include this information in the cover letter (see above). 
> Or you could write little bash script which shows how this works

But to answer your question: this was just a suggestion to save you explaining 
in text what each file does. If I assume correctly, something like

#!/bin/sh
#
clang-format -i -style=.clang-format test.c > _test.c
if cmp -s _test.c correct.c"
then
   echo "test.c was transformed correctly"
else
   echo "test.c was transformed incorrectly"
fi

should explain what the two .c files do and are intended for. Note that I 
didn't test this (I am not familiar with the clang-format syntax). 

Cheers
Lars
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel