Hey Peter,
Peter Hutterer said the following on 11/07/2012 02:34 AM:
> show-svg-image.c: In function ‘on_expose_cb’:
> show-svg-image.c:238:18: warning: variable ‘expose_event’ set but not used
> [-Wunused-but-set-variable]
> looks like one can just drop it, that's what I did here after merging.
>
> All four patches merged, thank you.
Thanks! :-)
> One comment I have is regarding commit messages: the prefix ("lib:", "test:"
> etc) is for guidance and not strictly required. What git somewhat enforces
> though is one line for summary, a blank line, then detailed descriptions.
>
> So looking at patch 4/4 (actual patch, for some reason the email subject
> lines are swapped) you have data: blah and test: blah in the commit message.
Yeah, I do send these by hand and of course that can fail. The 3rd
patch beign the 4th is just /me copying the wrong subject...
> This isn't necessary, just pick the one where the main change happens
> (usually lib or data, since that is the meat of the repository). Ideally
> there's one logical change (in this case add the data) and various extra
> bits that accompany it (in this case testing)
>
> OTOH, patch 3/4 introduces both an XML validator and a image display tool.
> IMO these should be in two different patches to make tracking these changes
> easier, and they are two, not one, logical changes. I've split them up here
> locally, so no further patch sets required.
Agreed.
> Also, last comment: if the rules above seem arbitrary, they pretty much are,
> sorry:) The best rule to follow is the "one logical change per patch".
No problem with rules, each project has its rules and when
contributing to different projects, one has to adapt ;-)
What I usually do is to (try to) do just like what is seen in the past
log, well, sometimes I either don't get it right, make a mistake or
look at a wrong place. That happens.
> (I've also added the two new binaries to .gitignore)
Thanks again for merging these patches, that'll definitely help for
the GNOME OSD window.
Cheers,
Olivier.
------------------------------------------------------------------------------
LogMeIn Central: Instant, anywhere, Remote PC access and management.
Stay in control, update software, and manage PCs from one command center
Diagnose problems and improve visibility into emerging IT issues
Automate, monitor and manage. Do more in less time with Central
http://p.sf.net/sfu/logmein12331_d2d
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
