[mythtv] Ticket #3910: Mythshutdown cleanup

Matthew Wire devel at mrwire.co.uk
Sat Jan 10 10:36:22 UTC 2009


Daniel,

Thankyou for taking the time to review this patch.  I understand about
the isRunning stuff although I'm not aware of any platform independent
way of detecting if processes are running?

As for the changes to mythdb get/set settings code I am hoping you may
reconsider.  It was originally designed to save a few lines of code
within mythshutdown but quickly grew out of that when I realised that
there was a whole load of code duplication within the dbsettings code.
The patch looks a lot more complex than it actually is because of the
way it has been generated (00-3910-mythdb-dbsettings.patch).

What it actually does is modify the settings code in GetHostSetting so
that it can retrieve settings from the global table only, host table
only or host first then global (as previous).  This allows the 99%
duplicate code in GetSetting to be removed, as well as the code in
mythshutdown.

I do appreciate that you don't want to destabilise a core method but I
think in this case it is worth it.  If people are running trunk they
should expect things to break occasionally anyway :)
In the long term this change makes the code more maintainable, easier to
modify for a mythdb daemon etc. (as has been mentioned as a possibility
in IRC) and cleaner.  I have been running this code for about six months
now and would put money on it not breaking anything.
People should be backing up their databases before upgrading anyway so a
quick email to the dev list warning people that database settings access
code has changed and an svn revert on the patch if there are any
problems!

Cheers,
Matthew


Le mardi 30 décembre 2008 à 23:23 +0000, MythTV a écrit :
> #3910: Mythshutdown cleanup
> -----------------------------------------------+----------------------------
>  Reporter:  Matthew Wire <devel at mrwire.co.uk>  |        Owner:  danielk
>      Type:  patch                              |       Status:  closed 
>  Priority:  minor                              |    Milestone:  unknown
> Component:  mythtv                             |      Version:  head   
>  Severity:  medium                             |   Resolution:  wontfix
>   Mlocked:  0                                  |  
> -----------------------------------------------+----------------------------
> Changes (by danielk):
> 
>   * status:  assigned => closed
>   * resolution:  => wontfix
> 
> 
> Comment:
> 
>  Matthew, I'm sorry you have put so much effort into this. I'm not going to
>  apply the isRunning patch because moving it to util.h might encourage
>  others to use it, and it is just not a good idea (see the comment I added
>  to it in [19505].) And the DB settings code changes may be fine, but I
>  don't think the small code savings in mythshutdown are worth a potentially
>  destabilizing change to one of the core methods of MythContext. I hope
>  this isn't too discouraging, we really like it when people send patches
>  and keep them up to date to boot!
> 



More information about the mythtv-dev mailing list