Re: [PATCH v5 0/3] Dell Airplane Mode Switch driver

[Darren Hart]

On Mon, Jun 08, 2015 at 09:30:36AM +0200, Pali Rohár wrote:
> On Sunday 07 June 2015 21:12:38 Darren Hart wrote:
> > On Sat, Jun 06, 2015 at 10:23:27AM +0200, Pali Rohár wrote:
> > > This patch series add new acpi Dell Airplane Mode Switch driver (DELLABCE and
> > > DELRBTN acpi devices). It provides radio HW switch events (together with current
> > > state of radio devices) and export them via rfkill interface. These events are
> > > also used in dell-laptop driver instead i8042 filter hook function (when acpi
> > > device is available).
> > > 
> > > In v2 was added support for laptops which have toggle button instead HW slider
> > > switch. For that I reused code done by Alex Hung.
> > > 
> > > v3 is just cleanup.
> > > 
> > > In v4 was fixed dependency problems.
> > 
> > Please be more specific in the future. You want people that haven't looked at
> > this for a couple of weeks to be able to quickly identify what changed and
> > confirm they agree. You want people to review your code, you want to reduce the
> > barrier to do so. Easy to do while it's fresh in your mind.
> > 
> > Thanks for the update, I have these qeueud to testing.
> > 
> 
> I wrote more details into individual patches above diffstat lines. But
> if you want detailed description also to cover letter email, I can do
> that in the future...

The individual patches is better. It's typical to leave the entire changelog
after the ---, and not just the current version. I found what I was actually
looking for was in the v4 changelog.

It's not a huge deal, but if the goal is to get people (and not just me) to
review the code, we want to make it as easy for them as possible to get the
necessary context to start reviewing. No doubt certain maintainer may feel
differently.

-- 
Darren Hart
Intel Open Source Technology Center
--
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/