Re: [PATCH] mfd: (max8997) Handle the potential error formfd_add_devices

From: Lee Jones
Date: Mon Dec 16 2013 - 11:36:00 EST


This is not a good introduction to the Kernel Community.

Please adapt your attitude or people will stop helping you.

> >> I think you commented on the wrong patch. There has been a newer submitted.
> >
> > No top posting please.
>
> Tell that to the client I need to use. IMO, making these inline posts
> mandatorily when the reply is a single line makes not much sense.
> Anyway, I will follow the inconvenient way.

If you are not replying to a particular comment, then there is no need
to quote it.

Please read and inwardly digest:
Documentation/email-clients.txt

> >> > The $SUBJECT line is wrong. To see how a subsystem usually formats
> >> > theirs you must do something like `git log --oneline -- <subsystem>`.
> >> > And duplicate the format.
> >> >
> >> > Commit message?
> >
> > These comments are still relevant, please re-post your patch with the
> > points rectified.
>
> I really do not understand how they relevant. "Commit message?" ->
> What about it?

The issue is that there isn't one.

> It has a pretty clear commit message.

If you are referencing my comments about the $SUBJECT line, then I
have to disagree with you there. It's actually pretty vague, does not
describe either the issue or what steps you've taken to rectify it.

> Are you now just
> picking nits about "foo:" vs "(foo)" in the short line?

That is also an issue. Did you issue the command I sent you:

`git log --oneline -- drivers/mfd`

Issue it now and see if _anyone_ has _ever_ used your formatting.

> >> >> + if (ret < 0) {
> >> >> + dev_err(dev, "cannot add mfd cells\n");
> >> >> + goto err_mfd;
> >> >> + }
> >> >
> >> > Have you tested this patch on h/w? Did you even compile it?
> >
> > You must ensure to test your patches before sending to the MLs, it's
> > the very least we expect.
>
> I am not sure what point you are trying to make. Feel free to reject
> the patch for this error handling.

I'm not rejecting it because of the error handling, I'm rejecting it
because it hasn't been tested and it doesn't even compile.

> Clearly, the patch has been updated
> due to a previous mistake. I would not make a fuss about an issue
> which had been fixed before getting any comment.

How was this 'clear'? Our inboxes are date/time sequential.

This patch was read _before_ the one you posted _subsequently_.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/