[asterisk-dev] [svn-commits] crichter: branch 1.2 r72087 - /branches/1.2/channels/misdn/

[Eric "ManxPower" Wieling]

Christian wrote:
> Russell Bryant wrote:
>> SVN commits to the Digium repositories wrote:
>>   
>>> Author: crichter
>>> Date: Wed Jun 27 04:26:53 2007
>>> New Revision: 72087
>>>
>>> URL: http://svn.digium.com/view/asterisk?view=rev&rev=72087
>>> Log:
>>> simplified channel finding and locking a lot. removed unnecessary #ifdefed areas.
>>>     
>> Judging by this commit message, these types of changes really aren't in line 
>> with our standards for what should be changed in release branches.  The general 
>> rule is to only change release branches to fix specific known bugs.  Changes 
>> that you could classify as "code cleanup" are generally not appropriate.
>>
>>   
> OK, you're right in general
> 
> but in this specific case i'm trying to fix known issues, unfortunately 
> when coming across "needed code cleanup", i just do it ;) .. anyway i'll 
> be more careful in differentiating code cleanups and bug fixes.
> 
> regarding the simplifications, it is necessary to do them, because there 
> are known issues that need a fix, if i don't simplify some specific code 
> areas i'll be forced to make a fix in multiple positions in the file, 
> which on the other hand might cause bugs again ..
> 
>> If you are ever not sure if something is appropriate for changing in release 
>> branches, just bring it up on this list and others will be happy to weigh in 
>> with thoughts and opinions.

The real problem here is that the more code you change the higher the 
chances are that it will break something in some horrible way.

My current pet peeve is the code change that happened somewhere between 
1.2.15 and 1.2.17 that caused asterisk to SEGFAULT when running 
app_macro under some situations.  The change should not have caused 
that, but it did.    That bug was fixed in 1.2.19, IIRC.