On Tue, Nov 24, 2009 at 09:20:38PM +0000, Przemysław Firszt wrote:
> From 4cf5a249e2abc489ce671a277d72ec14af202392 Mon Sep 17 00:00:00 2001
> From: Przemo Firszt <[email protected]>
> Date: Tue, 24 Nov 2009 20:50:39 +0000
> Subject: [PATCH 1/2] Duplicate comment removed
> 
> ---
>  src/xf86Wacom.c |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> index 6b76039..5b8778d 100644
> --- a/src/xf86Wacom.c
> +++ b/src/xf86Wacom.c
> @@ -435,12 +435,6 @@ void xf86WcmInitialCoordinates(LocalDevicePtr local, int 
> axes)
>       return;
>  }
>  
> -/*****************************************************************************
> - * xf86WcmRegisterX11Devices --
> - *    Register the X11 input devices with X11 core.
> - 
> ****************************************************************************/
> -
> -
>  #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) < 5
>  /* Define our own keymap so we can send key-events with our own device and 
> not
>   * rely on inputInfo.keyboard */
> -- 
> 1.6.5.2

Merged, thanks.

Ping - this is personal preference but I notice that most of these comments
serve little purpose, either they're not enough to actually explain what the
function does or they're quite obvious by the function name.
In addition, the comment style changes between the files.
I'm not adverse to having proper doxygen comments instead, but I'm also not
adverse to simply remove comments like (fake example):
/* xf86WcmAddTwoNumbers - adds two numbers */

Any comments?

> From acf166c340333efc3303ce836501c2bb5af3fa06 Mon Sep 17 00:00:00 2001
> From: Przemo Firszt <[email protected]>
> Date: Tue, 24 Nov 2009 20:57:31 +0000
> Subject: [PATCH 2/2] Comment fixed
> 
> ---
>  src/xf86Wacom.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/xf86Wacom.c b/src/xf86Wacom.c
> index 5b8778d..160fd60 100644
> --- a/src/xf86Wacom.c
> +++ b/src/xf86Wacom.c
> @@ -581,7 +581,7 @@ static struct { KeySym keysym; CARD8 mask; } keymod[] = {
>  #endif
>  
>  
> /*****************************************************************************
> - * xf86WcmInitialprivSize --
> + * xf86WcmInitialToolSize --
>   *    Initialize logical size and resolution for individual tool.
>   
> ****************************************************************************/
>  
> -- 
> 1.6.5.2

Przemo - please use the git commit messages as a tool to convey the reason
behind the patch (not necessarily the method, it's often obvious).
git commits are cheap, git log is cheap and I find myself reading log files
more often than the actual code they change. With hundreds of commits and
several repositories to keep track of, it becomes quite important to be able
to find commits and _understand_ commits quickly. as a general rule, if you
were to just send the patch, the commit message should convey enough
information why one should review and apply the patch - without even looking
at the source.

commit messages like "comment fixed" have little meaning in that regard. In
this case something more explanatory is 
"Fix comment - xf86WcmInitalprivSize is actually xf86WcmInitialToolSize"
or, for the other patch
"Remove duplicate comment for xf86WcmRegisterX11Devices.

This function still exists, a duplicate comment got accidentally added in
425351f22821ade814f7f6412d18c1eb25904661."

Of course, I am being extra-picky here. In these cases it is obvious and the
only change comments, but it's a good habit to get into. think of those poor
developers who have too little time ;)

That aside, thanks for the patches, I've merged them both (with extended
modified commit messages) and will push them in a tick.

Cheers,
  Peter

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to