Re: [PATCH 1/2] i2c-i801: Handle multiple instances instead of keeping global state

[David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]

On Sun, 2010-10-31 at 10:19 +0100, Jean Delvare wrote:
> On Sat, 30 Oct 2010 19:47:58 -0400, David Woodhouse wrote:
> > On Sun, 2010-10-31 at 00:39 +0100, Ben Dooks wrote:
> > > On Sat, Oct 30, 2010 at 07:34:23PM -0400, David Woodhouse wrote:
> > > > On Sat, 2010-10-30 at 18:24 +0200, Jean Delvare wrote:
> > > > > An explanation why this change is needed would be nice.
> > > > 
> > > > Um, does it really need explaining? It's really poor form to keep driver
> > > > state in global variables rather than per-instance, even if you *don't*
> > > > actually have more than one device.
> 
> This is actually a good second half for an explanation paragraph.
> 
> > > I always like to fill it in, it makes it easier for lazy folks who can't
> > > be bothered to read the patch itself.
> > 
> > The explanation fits in one line:
> >   "Handle multiple instances instead of keeping global state"
> 
> No, this isn't en explanation. This is a statement of _what_ your patch
> does. This is needed but not sufficient. What is additionally requested
> from you is an explanation _why_ it does it.
> 
> > I could try adding a paragraph explaining further that this patch will
> > make the driver handle multiple instances of the device, by keeping its
> > state per-device instead of in global variables. But that seems
> > somewhat... redundant.
> 
> You want to explain that the assumption of the original driver author
> that there can be only one supported device with one SMBus interface on
> a given system no longer holds true for new devices to be released soon.

Pfft. That was covered by this line in the patch itself:

-/* Note: we assume there can only be one I801, with one SMBus interface */

I cannot imagine a realistic case where any further comment in the
changelog would really be useful -- where a competent person glancing at
the resulting commit would be in *any* doubt as to what was being done
and why. But since it's easier to provide a redundant 'explanation' than
to argue about it, I'll include that in the next version too.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org                              Intel Corporation