lkml.org 
[lkml]   [2017]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Support new Alps HID Touchpad device
On Tue, 12 Sep 2017, Masaki Ota wrote:

> This is the patch for support new Alps HID Touchpad device.
> I submitted these patch before, but it was not completed.
> So I separate the patch to some parts and release it again.

Hi,

I have a few minor comments -- as in the past, I'd like to ask for
improved changelogs, so that they are up to our kernel standards. As you
are a regular contributor, it's time to start getting things right :)

Namely:

- please be generally more verbose / detailed in your changelogs
- explain not only what changes are being done, but also why they are
being done (what is the motivation). As an example, patch #1 has this:

-To support Alps T4 device, clean up the source code
-Delete unnecessary structure

Please briefly document the cleanups, and also document which structure
is being removed and why it is redundant
- you can drop the repeating "Minor changes in hid-alps.c for support new
Alps device" prefix for the patche subjects, as it doesn't really
provide any extra information
- pathces #4 and #5 do quite a substantial changes to the code, it'd be
nice to describe in a few sentnces what are the changes (what are the
specifics of the new protocol, etc)

Thanks a lot!

--
Jiri Kosina
SUSE Labs

\
 
 \ /
  Last update: 2017-09-16 02:27    [W:1.491 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site