[OPW,kernel] This patch fixes space after if statement in nvec/nvec.c
diff mbox

Message ID 20131002170425.GA17622@gmail.com
State Accepted
Headers show

Commit Message

Tugce Sirin Oct. 2, 2013, 5:04 p.m. UTC
Well, I'm quite excited. I hope this looks good.

This patch fixes space after if statement in nvec/nvec.c
Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
---
 drivers/staging/nvec/nvec.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg KH Oct. 2, 2013, 5:28 p.m. UTC | #1
On Wed, Oct 02, 2013 at 08:04:31PM +0300, Tugce Sirin wrote:
> Well, I'm quite excited. I hope this looks good.

It does, the patch itself is great, but I have a few minor comments
about the "metadata" around the patch, that will make accepting your
patches easier in the future.

First off, this line you wrote, doesn't need to be here, otherwise it
would show up in the kernel changelog.  Comments about patches go below
the "---" line in the patch, as git will strip them out, and yet we can
still see them in your email.

Second the subject could be a bit more descriptive as to what part of
the kernel you are making a change for.  This line will show up in the
kernel changelog as part of the short summary and needs to be able to
narrow it down to where the change is for.

So, for this specific patch, I would recommend a subject like:
	Subject: [PATCH] staging: nvec: fix space after if statement issue

> 
> This patch fixes space after if statement in nvec/nvec.c
> Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>

This is great, but you need an extra line after the sentance and before
the "Signed-off-by:" line for the tools to pick things up properly (odds
are it will still work, but convention relies on an extra line just to
make it easier for humans to read it.

> ---
>  drivers/staging/nvec/nvec.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> index 5a5c639..3066ee2 100644
> --- a/drivers/staging/nvec/nvec.c
> +++ b/drivers/staging/nvec/nvec.c
> @@ -802,7 +802,7 @@ static int tegra_nvec_probe(struct platform_device *pdev)
>  		unmute_speakers[] = { NVEC_OEM0, 0x10, 0x59, 0x95 },
>  		enable_event[7] = { NVEC_SYS, CNF_EVENT_REPORTING, true };
>  
> -	if(!pdev->dev.of_node) {
> +	if (!pdev->dev.of_node) {

The patch is great, and I'll go queue it up soon and you will get an
automated email from my scripts as to what is going to happen to it.

I'll edit the Subject and body to be correct, but in the future, I'd
really like to not do that, as maintainers don't have the spare time to
do that, and you want to make it as easy as possible for your patch to
be accepted, don't give me _any_ reason to reject a patch.

thanks,

greg k-h
Tugce Sirin Oct. 2, 2013, 7:40 p.m. UTC | #2
Thank you for your great feedback. I'll be more carefull for the next patch
(patches I hope :) )

Best regards,
Tugce Sirin


2013/10/2 Greg KH <gregkh@linuxfoundation.org>

> On Wed, Oct 02, 2013 at 08:04:31PM +0300, Tugce Sirin wrote:
> > Well, I'm quite excited. I hope this looks good.
>
> It does, the patch itself is great, but I have a few minor comments
> about the "metadata" around the patch, that will make accepting your
> patches easier in the future.
>
> First off, this line you wrote, doesn't need to be here, otherwise it
> would show up in the kernel changelog.  Comments about patches go below
> the "---" line in the patch, as git will strip them out, and yet we can
> still see them in your email.
>
> Second the subject could be a bit more descriptive as to what part of
> the kernel you are making a change for.  This line will show up in the
> kernel changelog as part of the short summary and needs to be able to
> narrow it down to where the change is for.
>
> So, for this specific patch, I would recommend a subject like:
>         Subject: [PATCH] staging: nvec: fix space after if statement issue
>
> >
> > This patch fixes space after if statement in nvec/nvec.c
> > Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
>
> This is great, but you need an extra line after the sentance and before
> the "Signed-off-by:" line for the tools to pick things up properly (odds
> are it will still work, but convention relies on an extra line just to
> make it easier for humans to read it.
>
> > ---
> >  drivers/staging/nvec/nvec.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> > index 5a5c639..3066ee2 100644
> > --- a/drivers/staging/nvec/nvec.c
> > +++ b/drivers/staging/nvec/nvec.c
> > @@ -802,7 +802,7 @@ static int tegra_nvec_probe(struct platform_device
> *pdev)
> >               unmute_speakers[] = { NVEC_OEM0, 0x10, 0x59, 0x95 },
> >               enable_event[7] = { NVEC_SYS, CNF_EVENT_REPORTING, true };
> >
> > -     if(!pdev->dev.of_node) {
> > +     if (!pdev->dev.of_node) {
>
> The patch is great, and I'll go queue it up soon and you will get an
> automated email from my scripts as to what is going to happen to it.
>
> I'll edit the Subject and body to be correct, but in the future, I'd
> really like to not do that, as maintainers don't have the spare time to
> do that, and you want to make it as easy as possible for your patch to
> be accepted, don't give me _any_ reason to reject a patch.
>
> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups
> "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

Patch
diff mbox

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 5a5c639..3066ee2 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -802,7 +802,7 @@  static int tegra_nvec_probe(struct platform_device *pdev)
 		unmute_speakers[] = { NVEC_OEM0, 0x10, 0x59, 0x95 },
 		enable_event[7] = { NVEC_SYS, CNF_EVENT_REPORTING, true };
 
-	if(!pdev->dev.of_node) {
+	if (!pdev->dev.of_node) {
 		dev_err(&pdev->dev, "must be instantiated using device tree\n");
 		return -ENODEV;
 	}