From: Peter H. <pet...@wh...> - 2009-11-25 00:42:00
|
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 <pr...@fi...> > 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 <pr...@fi...> > 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 |