[MPlayer-dev-eng] [PATCH] factorize print_version()

[Uoti Urpala]

On Tue, 2009-01-13 at 00:36 +0100, Diego Biurrun wrote:
> On Tue, Jan 13, 2009 at 01:25:44AM +0200, Uoti Urpala wrote:
> > On Mon, 2009-01-12 at 23:51 +0100, Diego Biurrun wrote:
> > > On Tue, Jan 13, 2009 at 12:37:06AM +0200, Uoti Urpala wrote:
> > > > The code can be moved back later reasonably easily, but is it really
> > > > beneficial even in the short term? MEncoder is quite rotten already. As
> > > > long as MEncoder is not really maintained MPlayer quality is what
> > > > matters, and IMO it's worsened by changes like this.
> > > 
> > > I think the change is worth the trouble.
> > > 
> > > It will make my life easier when I fix bug 1378 and it will make my
> > > life easier when I update the copyright notice next year.  So I think
> > > it's beneficial in the short term and not a big deal in the long run.
> > 
> > That sounds like less benefit than even just the work needed to do the
> > change itself...
> 
> The work is done so the point is moot IMO.
> And to me it keeps sounding like the possible future trouble of moving
> code around again is even smaller than the current benefit.

Did you understand what I wrote about the negative effects? Those are
reasons why the effect of such changes on code quality is questionable
_now_, not just after some possible future changes. For a rarely
modified and used function like this the harm is small, but the benefits
are even smaller.

> > > If you think print_version() should be put in a different place, I'm
> > > all ears.  But basically anything that makes mplayer.c smaller cannot
> > > hurt IMO.
> > 
> > I do intend to split mplayer.c further at some point. But I have not yet
> > decided what the best way would be. Doing an arbitrary split completely
> > at random or based on whether the code has something in common with
> > mencoder.c is not an improvement.
> 
> Then suggest another place.  Failing that I do not see a reason to hold
> off on committing this much longer.

As long as all the related code is in mplayer.c, all other existing
files are worse choices. That's exactly what I meant above - you can't
just take arbitrary functions, put them in "some other file" and call
that an improvement.