On Fri, 22 Mar 2013 at 23:22:48 +0100, Daniel Déchelotte wrote:
>
>  * Because the patches were started long ago on a different computer,
>    and because wmaker's code based evolved in parallel, please bear
>    with the occasionally inconsistent coding style (spacing, brackets,
>    ...). Any tip to automatically reformat a set of patches is
>    appreciated.

Thanks for asking, and indeed the coding style of your patches is
very different.

You can run
        
        indent -linux -l115 file.c
        
and check the diff to see where your coding style is different from
the accepted in wmaker. You can also look at the style definition in

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle

if you have the time.

Also, if you have the kernel sources, you can run their script called
checkpatch.pl on the patches you generate using git format-patch.

That script seems to require running from inside the kernel tree, though.

Btw, many of your patches seem nice but they could be even nicer if they
had descriptions of what they do, their motivation etc. Simply dumping
the code around and expecting all the context can be grasped from the
subject: line is not nice to help someone who is deciding whether to
want the patch or not.

And even simple patches like the one changing the window list width
should have a description or motivation.

Patch descriptions in the wmaker repo should be considered as an
argument to try to sell me something :-)

For example, in your webpage you have a good description about the patch
"Avoid dock flickering". That is exactly the kind of information I like
to read in the commit message. You describe the problem and then write
a wonderfully simple "Explanation and correction:" which says it all.

After reading that and taking a brief look at the patch, I can simply
put it in the repository without any doubts. But that description _must_
be in the commit message, so that it will never be forgotten.

Another general comment I have is about the subject: line of your patches.
They are not optimal to convey the summary of the change. Think about
someone trying to find where a regression first appeared and taking a look
in a terminal window in the output of 'git log'. 

If you conclude that your subject line will not immediately tell him/her
that that patch is relevant or not in that particular regression, rewrite it.

One tip. If the patch touches only WPrefs, it's very helpful for the
regression scenario above if the patch starts with WPrefs:. For example,
patch number 5 could be

WPrefs: Consolidate two createImages() static functions into a global one


-- 
To unsubscribe, send mail to [email protected].

Reply via email to