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

[Lee Jones <lee.jones@linaro.org>]

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