Re: [PATCH] kconfig: untangle EXPERT and EMBEDDED

[Andrew Jones]

On Tue, Jan 17, 2012 at 12:54:01PM -0800, David Rientjes wrote:
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index a421abd..73c2d39 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -63,7 +63,7 @@ menu "Special HID drivers"
> >  config HID_A4TECH
> >         tristate "A4 tech mice" if EXPERT
> >         depends on USB_HID
> > -       default !EXPERT
> > +       default !EMBEDDED
> >         ---help---
> >         Support for A4 tech X5 and WOP-35 / Trust 450L mice.
> > 
> > and the other HID drivers...
> > 
> 
> Um, no, HID_A4TECH is still only configurable for CONFIG_EXPERT with this 
> patch.  Jerome's premise is that this should be configurable for 
> CONFIG_EMBEDDED instead.  Please read what he wrote.

Yes, you still need EXPERT to expose the option, but then EMBEDDED will
switch the default. You only need to set EMBEDDED=y to do that though,
that's what this little thing called "select" does.

> 
> When it's configurable only for CONFIG_EMBEDDED, then you can propose that 
> to the HID maintainers.  If they agree, then we don't care if users 
> currently with CONFIG_EXPERT=y and CONFIG_EMBEDDED=n lose the option, but 
> that needs to be handled on a case-by-case basis when breaking backwards 
> compatibility.

Why would you *EVER* want users that have CONFIG_EXPERT=y and
CONFIG_EMBEDDED=n to automatically, silently lose that option? IOW, why
was your patch, 6a108a14fa35, ever posted that way in the first place?!

Oh, so now we can break backwards compatibility for some cases? What is
the criteria for those cases? Let me guess at a few;

1. When there is a well documented transfer from old to new, possibly with
   a deprecation period.
2. To fix a bug where the documentation doesn't match the implementation,
   and the implementation is wrong. If users got used to that wrong
   implementation, then they're the ones at fault. The documentation
   was/is the contract. It would still be best to do something like (1)
   here, with a deprecation period, but depending on the case, that may
   not be necessary or desirable.
3. To restore sanity to the general kernel config options. Oh, wait,
   that's just (2) again.

> 
> > I guess it could be changed to 'if EXPERT || EMBEDDED', but at the moment
> > EMBEDDED selects EXPERT, so that's not currently necessary. I guess what's
> > above should be sufficient then. Oh, wait! That's exactly what this patch
> > does! And anybody who actually read it would have seen that.
> > 
> 
> One of many reasons why it's completely wrong, and is nacked.

Changing 'if EXPERT' to 'if EXPERT || EMBEDDED' for particular options is
*NOT* in the scope of this patch. Not doing it in this patch is not only
OK, but correct. Adding out-of-scope changes to patches is wrong.

> 
> > BTW, the HID maintainer, Jiri Kosina, is already on cc, since I cc'ed
> > every maintainer of the files that this patch touches.
> > 
> 
> That type of attitude is a great way for your patches to be lost in 
> oblivion, you can't expect everyone on the cc list to be actively reading 
> this thread.  I've considered not reading it myself since it's pretty 
> pointless.

What do you mean you've considered not reading this pointless thread? You
wrote it! All the nonsense comes from you. Besides the patch submission,
which fixes a real problem, this thread HAS been pointless, and wasted a
lot of my time.

> If you wish to submit kconfig patches for options that touch 
> specific subsystems, you'll need to separate them out and propose them to 
> the individual subsystem maintainers.

This patch is for the general kernel. It wouldn't make sense to break it
up for each subsystem. It actually wouldn't even be possible to merge
those patches separately without risking config breakage on bisections. I
mentioned that in the commit message.
--
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/