[asterisk-dev] plea for more verbose commit messages (Re: [asterisk-commits] file: branch 1.4 r75712 - in /branches/1.4: apps/ channels/ pbx/)

Luigi Rizzo rizzo at icir.org
Wed Jul 18 15:17:09 CDT 2007


On Wed, Jul 18, 2007 at 08:00:23PM -0000, SVN commits to the Asterisk project wrote:
> Author: file
> Date: Wed Jul 18 15:00:23 2007
> New Revision: 75712
> 
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=75712
> Log:
> Backport GCC 4.2 fixes. Without these Asterisk won't build under devmode using GCC 4.2.

commit messages are also a way for others to learn something from your
experience. So I would like to encourage committers to be a bit
more verbose in their log messages, especially if you think the
issues might have an impact on other code (eg. third party stuff,
or patches yet to be committed, etc.)

For instance, in this case it might be useful to know what gcc 4.2 was
complaining about, as e.g. the previous code in app_voicemail.c
seemed correct. Perhaps uninitialized variables ? Why then previous
compilers did not complain ? Does it depend on the -O level or
is it because of a gcc 4.2 bug ?

Another case was the previous conversion from strftime/tm to
ast_strftime/ast_tm - it would be good to add, in the commit message,
a short recipe for the conversion (in this specific case it is
probably "replace strftime with ast_strftime, struct tm with struct
ast_tm, time_t x with struct timeval x (and check x.tv_sec instead
of just x)".
To you, who made the change, it won't take long to document it, but
for others this is precious to avoid mistakes and also cross-check your
changes.

	thanks
	luigi

> Modified:
>     branches/1.4/apps/app_voicemail.c
>     branches/1.4/channels/chan_agent.c
>     branches/1.4/channels/chan_sip.c
>     branches/1.4/pbx/pbx_realtime.c
> 
> Modified: branches/1.4/apps/app_voicemail.c
> URL: http://svn.digium.com/view/asterisk/branches/1.4/apps/app_voicemail.c?view=diff&rev=75712&r1=75711&r2=75712
> ==============================================================================
> --- branches/1.4/apps/app_voicemail.c (original)
> +++ branches/1.4/apps/app_voicemail.c Wed Jul 18 15:00:23 2007
> @@ -553,12 +553,9 @@
>  	ast_copy_flags(vmu, (&globalflags), AST_FLAGS_ALL);	
>  	if (saydurationminfo)
>  		vmu->saydurationm = saydurationminfo;
> -	if (callcontext)
> -		ast_copy_string(vmu->callback, callcontext, sizeof(vmu->callback));
> -	if (dialcontext)
> -		ast_copy_string(vmu->dialout, dialcontext, sizeof(vmu->dialout));
> -	if (exitcontext)
> -		ast_copy_string(vmu->exit, exitcontext, sizeof(vmu->exit));
> +	ast_copy_string(vmu->callback, callcontext, sizeof(vmu->callback));
> +	ast_copy_string(vmu->dialout, dialcontext, sizeof(vmu->dialout));
> +	ast_copy_string(vmu->exit, exitcontext, sizeof(vmu->exit));
>  	if (maxmsg)
>  		vmu->maxmsg = maxmsg;
>  	vmu->volgain = volgain;
> 



More information about the asterisk-dev mailing list