Opened 14 years ago
Closed 12 years ago
#3910 closed patch (wontfix)
Mythshutdown cleanup
| Reported by: | Owned by: | danielk | |
|---|---|---|---|
| Priority: | minor | Milestone: | unknown |
| Component: | mythtv | Version: | head |
| Severity: | medium | Keywords: | mythshutdown mythcontext isrunning globalsetting |
| Cc: | Ticket locked: | no |
Description
3 patches:
Add getGlobalSetting and setGlobalSetting into mythcontext.cpp.
Add isRunning to util.cpp.
Remove functions from mythshutdown and use new library ones.
Attachments (15)
Change History (25)
Changed 14 years ago by
| Attachment: | libmyth-mythcontext_globalSetting-31082007.diff added |
|---|
Changed 14 years ago by
| Attachment: | mythshutdown-31082007.diff added |
|---|
mythshutdown using new library functions
comment:1 follow-up: 2 Changed 14 years ago by
I'm trying to work on some shutdown/wakeup features at the moment and part of this includes centralising all the shutdown/wakeup functions into a shutdownwakeup class within libmyth. The changes on this patch facilite part of this.
comment:2 Changed 14 years ago by
Replying to Matthew Wire <devel@mrwire.co.uk>:
I'm trying to work on some shutdown/wakeup features at the moment and part of this includes centralising all the shutdown/wakeup functions into a shutdownwakeup class within libmyth. The changes on this patch facilite part of this.
Can you be a little more specific about where you are going with all this. What is the final goal? I'm not sure Isaac and the other devs will want to add extra stuff to libmyth unless there is a very good reason to do so.
Changed 14 years ago by
| Attachment: | libmyth-mythcontext_globalSettings-02092007.diff added |
|---|
Add saveGlobalSetting function to mythcontext
Changed 14 years ago by
| Attachment: | libmyth-util_isRunning02092007.diff added |
|---|
Add isRunning function to util.cpp
Changed 14 years ago by
| Attachment: | mythshutdown-02092007.diff added |
|---|
Remove setGlobalSetting and getGlobalSetting and use mythcontext instead
comment:3 Changed 14 years ago by
Sorry, that comment was rather vague. I'll follow up with an email on the dev list. That'll teach me to submit late at night.
I've also refreshed all three patches. The mythcontext patch is now only four lines since I realised that the existed functions could be re-used with one wrapper function.
libmyth-mythcontext_globalSettings adds saveGlobalSetting function to mythcontext since there is currently no function that will do this.
libmyth-util_isRunning moves the isRunning function from mythshutdown to util.cpp. This function could be useful elsewhere in myth (this is the only part of the patch that I need for the shutdown/wakeup class).
mythshutdown removes getGlobalSetting and setGlobalSetting from mythshutdown and uses equivalent functions in mythcontext instead - reducing unnecessary code duplication. It also adds a dependency on util.h for the isRunning function.
Changed 13 years ago by
| Attachment: | mythshutdown-util-isRunning-18112007.patch added |
|---|
Move isRunning function to libmyth/util.cpp
Changed 13 years ago by
| Attachment: | mythshutdown-mythcontext-get_setSettings-18112007.patch added |
|---|
Use functions in mythcontext instead of copies for get/set Settings
Changed 13 years ago by
| Attachment: | #3910 mythshutdown-mythcontext-get_setSettings-11122007.patch added |
|---|
Patch refresh for latest svn
Changed 13 years ago by
| Attachment: | #3910 mythshutdown-util-isRunning-11122007.patch added |
|---|
Patch refresh for latest svn
comment:4 Changed 13 years ago by
Updated patches for latest svn.
The mythcontext patch removes duplicate database set/get methods from mythshutdown and uses the ones in mythcontext instead.
The util patch moves the isRunning function from mythshutdown to libmyth so it can be used for other things (such as #4184).
Neither of these patches provide any new functionality in themselves, they are code cleanups which make future patches easier.
comment:5 Changed 13 years ago by
| Owner: | changed from Isaac Richards to danielk |
|---|---|
| Status: | new → assigned |
| Version: | unknown → head |
comment:6 Changed 13 years ago by
Matthew, I don't think getGlobalSetting("MythShutdownLock?", "0") and gContext->GetSetting?("MythShutdownLock?", "0") are equivalent the last one will prefer the setting for the local host if one is available. New setups shouldn't have any local host settings for that setting but my ancient DB on my dev box certainly does have - probably left over from some long gone version of Myth.
Changed 12 years ago by
| Attachment: | 05-3910-mythshutdown-util-isRunning.2.patch added |
|---|
Updated for trunk
Changed 12 years ago by
| Attachment: | 07-3910-mythdb-dbsettings-cleanup.patch added |
|---|
Cleanup of database access functions
Changed 12 years ago by
| Attachment: | 06-3910-mythshutdown-dbsettings.patch added |
|---|
Removal of database access functions from mythshutdown
comment:7 Changed 12 years ago by
Updated all patches for trunk and refactored where necessary.
07-3910-mythdb-dbsettings-cleanup.patch cleans up mythdb settings access code, reducing two almost identical functions down to a single function that does all the database access (GetSettingOnHost?). In it's default mode (tryGlobal is true) it will behave as before and look for a host setting before falling back to the global settings table (Paul, this fixes your issue that you raised). GetGlobalSetting? and SaveGlobalSetting? are added as wrapper functions for use by mythshutdown.
06-3910-mythshutdown-dbsettings requires the above patch. This patches mythshutdown to remove it's own copies of the Get/Save? settings code and use the ones in mythdb instead.
Changed 12 years ago by
| Attachment: | 21-3910-mythshutdown-dbsettings.patch added |
|---|
Changed 12 years ago by
| Attachment: | 22-3910-mythshutdown-util-isRunning.2.patch added |
|---|
comment:8 Changed 12 years ago by
Updated patches (21 and 22) so they apply cleanly.
Note: mythshutdown patch on #5935 should be applied before these two.
Note(2): mythdb-dbsettings-cleanup should be applied before mythshutdown-dbsettings.
comment:9 Changed 12 years ago by
comment:10 Changed 12 years ago by
| Resolution: | → wontfix |
|---|---|
| Status: | assigned → closed |
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!
setGlobalSetting and getGlobalSetting functions