Re: [PATCH] INITEST: Documentation: Explains how INITEST works.

[Amit Kumar]

Sorry, kernelnewbies address has been added by git-send-email. I was
testing
by sending to myself. I sent this patch as a test patch on this mailing
list before and downloaded.
I'll be cautious in future to review  recipient list.


On Wed, Oct 5, 2016, 5:00 PM Amit Kumar  wrote:

>
> This is a test patch.
>
> Signed-off-by: Amit Kumar 
>
> ---
> Is this patch in right format?
>
>  Documentation/initest.txt | 7 +++
>  1 file changed, 7 insertions(+)
>  create mode 100644 Documentation/initest.txt
>
> diff --git a/Documentation/initest.txt b/Documentation/initest.txt
> new file mode 100644
> index 000..eed4124
> --- /dev/null
> +++ b/Documentation/initest.txt
> @@ -0,0 +1,7 @@
> +INITEST
> +
> +1) This is the same user space program /sbin/init, but for the purpose of
> testing kernel system calls.
> +
> +2) It will also use testing drivers to test internal kernel API.
> +
> +3) This is a test patch.
> --
> 2.7.4
>
>
>
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] INITEST: Documentation: Explains how INITEST works.

[Amit Kumar]

On Fri, Sep 30, 2016, 1:22 PM  wrote:

> On Fri, 30 Sep 2016 07:49:50 +0200, Bjørn Mork said:
>
> > 1) There should be *some* body text describing the patch, even for a very
> >basic change which is explained in full in subject.  This goes first
> >in the body, separated from the tags with a single empty line.
>
> And to add to what he said:
>
> Explaining the *why* is better than the *how*.
>
> "Add Documentation/initest.txt" isn't that helpful - we can see from the
> diff
> that's what it does.  "Add documentation explaining how to deal with the
> Frobnizz  3000's wonky interrupts" is a lot better.
>
> Similarly, "fix off-by-one error" is OK - but extending it to "fix
> off-by-one
> error that causes the wireless card to select the wrong channel" is a lot
> better.
>
> Sell us that patch - tell us *why* we want it in the kernel, and why we
> should
> spend time reviewing it
>
> Please, you should not bother to review this patch seriously. As I have
> written in

this patch that "this patch is a test patch", not real one. It has been
answered by Mr. Bjorn Mork
and you. This patch was merely to check whether my patch format is
acceptable to the kernel
community.
-- 

Regards,
Amit Kumar
Twitter: @freeark1
Only Numbers Can Command.
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] INITEST: Documentation: Explains how INITEST works.

[Valdis . Kletnieks]

On Fri, 30 Sep 2016 07:49:50 +0200, Bjørn Mork said:

> 1) There should be *some* body text describing the patch, even for a very
>basic change which is explained in full in subject.  This goes first
>in the body, separated from the tags with a single empty line.

And to add to what he said:

Explaining the *why* is better than the *how*.

"Add Documentation/initest.txt" isn't that helpful - we can see from the diff
that's what it does.  "Add documentation explaining how to deal with the
Frobnizz  3000's wonky interrupts" is a lot better.

Similarly, "fix off-by-one error" is OK - but extending it to "fix off-by-one
error that causes the wireless card to select the wrong channel" is a lot 
better.

Sell us that patch - tell us *why* we want it in the kernel, and why we should
spend time reviewing it



pgpAZ06omSSoP.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] INITEST: Documentation: Explains how INITEST works.

[Amit Kumar]

Hi,

Thank you for your reply.
I'll follow your suggestion.

On Fri, Sep 30, 2016, 11:19 AM Bjørn Mork <bj...@mork.no> wrote:

> Amit Kumar <free.amit.ku...@gmail.com> writes:
>
> > Signed-off-by: Amit Kumar <free.amit.ku...@gmail.com>
> >
> > This is a test patch.
> > Is this patch in right format?
> > ---
> >  Documentation/initest.txt | 7 +++
> >  1 file changed, 7 insertions(+)
> >  create mode 100644 Documentation/initest.txt
> >
> > diff --git a/Documentation/initest.txt b/Documentation/initest.txt
> > new file mode 100644
> > index 000..eed4124
> > --- /dev/null
> > +++ b/Documentation/initest.txt
> > @@ -0,0 +1,7 @@
> > +INITEST
> > +
> > +1) This is the same user space program /sbin/init, but for the purpose
> of testing kernel system calls.
> > +
> > +2) It will also use testing drivers to test internal kernel API.
> > +
> > +3) This is a test patch.
>
>
> Looks pretty good.  But a few more things to keep in mind:
>
> 1) There should be *some* body text describing the patch, even for a very
>basic change which is explained in full in subject.  This goes first
>in the body, separated from the tags with a single empty line.
>
> 2) Signed-off-by and other tags go at the end of the body, right before
>   the "---" separator.  Don't put anything between the SOB and the
>   separator.  I realize that this was just a question that you wouldn't
>   make part of the submitted patch but still: It broke your example...
>
> 3) There is a place for questions about the patch which you do not want
>   to be part of the commit message: Right After the first "---"
>   separator.  Anything yout put there will be ignored by git when the
>   patch is applied, just like the diffstat git includes by itself.
>
>   This is useful if you need to explain something to the maintainer
>   regarding how to apply the patch, or if ytou have questions about it
>   which are irrelevant if applied.
>
>
>
> So the end result would be:
>
>
> Subject: [PATCH] INITEST: Documentation: Explains how INITEST works.
>
> This is a test patch.
>
> Signed-off-by: Amit Kumar <free.amit.ku...@gmail.com>
> ---
> Is this patch in right format?
>
>
> Documentation/initest.txt | 7 +++
> 1 file changed, 7 insertions(+)
> create mode 100644 Documentation/initest.txt
>
> diff --git a/Documentation/initest.txt b/Documentation/initest.txt
> [etc]
>
>
>
> Bjørn
>
-- 

Regards,
Amit Kumar
Twitter: @freeark1
Only Numbers Can Command.
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] INITEST: Documentation: Explains how INITEST works.

[Bjørn Mork]

Amit Kumar <free.amit.ku...@gmail.com> writes:

> Signed-off-by: Amit Kumar <free.amit.ku...@gmail.com>
>
> This is a test patch.
> Is this patch in right format?
> ---
>  Documentation/initest.txt | 7 +++
>  1 file changed, 7 insertions(+)
>  create mode 100644 Documentation/initest.txt
>
> diff --git a/Documentation/initest.txt b/Documentation/initest.txt
> new file mode 100644
> index 000..eed4124
> --- /dev/null
> +++ b/Documentation/initest.txt
> @@ -0,0 +1,7 @@
> +INITEST
> +
> +1) This is the same user space program /sbin/init, but for the purpose of 
> testing kernel system calls.
> +
> +2) It will also use testing drivers to test internal kernel API.
> +
> +3) This is a test patch.


Looks pretty good.  But a few more things to keep in mind:

1) There should be *some* body text describing the patch, even for a very
   basic change which is explained in full in subject.  This goes first
   in the body, separated from the tags with a single empty line.

2) Signed-off-by and other tags go at the end of the body, right before
  the "---" separator.  Don't put anything between the SOB and the
  separator.  I realize that this was just a question that you wouldn't
  make part of the submitted patch but still: It broke your example...

3) There is a place for questions about the patch which you do not want
  to be part of the commit message: Right After the first "---"
  separator.  Anything yout put there will be ignored by git when the
  patch is applied, just like the diffstat git includes by itself.

  This is useful if you need to explain something to the maintainer
  regarding how to apply the patch, or if ytou have questions about it
  which are irrelevant if applied.



So the end result would be:


Subject: [PATCH] INITEST: Documentation: Explains how INITEST works.

This is a test patch.

Signed-off-by: Amit Kumar <free.amit.ku...@gmail.com>
---
Is this patch in right format?


Documentation/initest.txt | 7 +++
1 file changed, 7 insertions(+)
create mode 100644 Documentation/initest.txt

diff --git a/Documentation/initest.txt b/Documentation/initest.txt
[etc]



Bjørn

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: [PATCH] INITEST: Documentation: Explains how INITEST works.

[Greg KH]

On Fri, Sep 30, 2016 at 10:47:09AM +0530, Amit Kumar wrote:
> Signed-off-by: Amit Kumar 
> 
> This is a test patch.
> Is this patch in right format?

What exactly do you mean by "right format"?  What do you want reviewed
here, the content of the patch, or the other stuff around the patch
(subject, changelog, etc.)?

thanks,

greg k-h

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

[PATCH] INITEST: Documentation: Explains how INITEST works.

[Amit Kumar]

Signed-off-by: Amit Kumar 

This is a test patch.
Is this patch in right format?
---
 Documentation/initest.txt | 7 +++
 1 file changed, 7 insertions(+)
 create mode 100644 Documentation/initest.txt

diff --git a/Documentation/initest.txt b/Documentation/initest.txt
new file mode 100644
index 000..eed4124
--- /dev/null
+++ b/Documentation/initest.txt
@@ -0,0 +1,7 @@
+INITEST
+
+1) This is the same user space program /sbin/init, but for the purpose of 
testing kernel system calls.
+
+2) It will also use testing drivers to test internal kernel API.
+
+3) This is a test patch.
-- 
2.7.4


___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies