[OpenSER-Devel] SF.net SVN: openser: [3442] trunk

[Dan Pascu]

On Wednesday 19 December 2007, Henning Westerholt wrote:
> > Well, even though you're busy I still think that the change helps
> > nothing and makes it harder for someone to make changes if they need
> > to.
>
> I think its help because one of the main problems for a medium sized
> code base like this is the clutter that gets accumulated over the time.
> This make maintainance and understanding of the codebase harder then
> necessary.

I still do not understand how a named constant improves things over a 
#define. Can you elaborate on this in the context on this change?

> If you had take a look into the actual change, you've noticed that the
> TIMEOUT define was actually located in line 429. This is not "on top of
> the file" for me.

Which means it was in a non-optimal position, but that can be improved. I 
cannot say the same about a named constant inside a function.

>
> If you maintain such a patch, and i have made your life a little bit
> harder now, then i'll revert this change. If this is not so, and you
> still can not accept my change, then we need to start a voting about
> this.

I don't maintain such a patch, and I'm not affected in any way by this 
change. Why I raised this issue is because you made a change that was 
adding little value (if any), while it potentially complicates things. If 
I do not voice my concerns now, tomorrow you may decide to change 
something else in a similar manner which would affect me. I think it 
would only be fair if you would consider such observations because others 
may have different views on how things should be done, and in the end I 
think it comes to how the person maintaining that code prefers to do 
things.

>
> But this perhaps is a little to much overhead for this type of change.

As I recall, the consensus was that changes to code for which you are not 
maintainer should get the approval of the maintainer unless obvious bug 
fixes. I do not recall there was any discussion why it would be better to 
replace #defines with named or even unnamed constants, so that people can 
state their views on the issue. I do not think this is a minor issue that 
only one person should start to make changes for without first hearing 
what others have to say about it.

-- 
Dan