Re: [PATCHv2 4/9] staging: atomisp: i2c: Disable non-preview configurations

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Fri, 16 Feb 2018 12:12:20 +0200
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Mauro,
> 
> On Wed, Feb 14, 2018 at 02:20:50PM -0200, Mauro Carvalho Chehab wrote:
> > Em Mon, 22 Jan 2018 13:31:20 +0100
> > Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> >   
> > > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > 
> > > Disable configurations for non-preview modes until configuration selection
> > > is improved.  
> > 
> > Again, a poor description. It just repeats the subject.
> > A good subject/description should answer 3 questions:
> > 
> > 	what?
> > 	why?
> > 	how?
> > 
> > Anyway, looking at this patch's contents, it partially answers my
> > questions:
> > 
> > the previous patch do cause regressions at the code.
> > 
> > Ok, this is staging. So, we don't have very strict rules here,
> > but still causing regressions without providing a very good
> > reason why sucks.
> > 
> > I would also merge this with the previous one, in order to place all
> > regressions on a single patch.  
> 
> It's trivial to bring back the configurations disabled here by just
> reverting this patch. The other patch does not disable any. That's why
> they're separate.

Yes, and I'm not saying otherwise.

The main issue here is that it lacks a description (as what's
there is just a copy of the subject).

Why is it needed to "disable non-preview configurations"?

Also, as you're actually commenting the code with #if 0, I'm assuming
that you're thinking on re-enable the code (or re-implement with a
different logic) in the future.

So, please add a note before the #if 0, as otherwise I'm pretty
sure someone will end by sending us patches just stripping it.

Regards,
Mauro