Review Request: Initial Kopete Logs Import Dialog

[Daniele Elmo Domenichelli]

> On Nov. 8, 2012, 2:57 a.m., David Edmundson wrote:
> > Why is this in the KDED? KDED Modules should always be really really tiny. This isn't.
> > It doesn't need to run throughout a KDE session, and prompts when you first log in aren't a good idea either.
> > 
> > I strongly think we should consider what the objectives actually are, and then reconsider where this goes.
> 
> Dan Vrátil wrote:
>     I agree that KDED module is not the best solution. 
>     
>     My idea was to have something like KResoure Migrator and KDED seemed like the best victim to abuse for that. We can't say "ok, let's do it on application startup", because we can't really know which KTp components users use and will start (unlike the KDED which is more or less guaranteed to run for every KTp user). 
>     
>     Would it be OK to have a custom small binary placed probably in common-internals, with autostart .desktop in /usr/share/autostart? After first run the application would disable itself:
>     
>         KSharedConfigPtr config = KGlobal::config();
>         KConfigGroup group( config, "Startup" );
>         group.writeEntry( "EnableAutostart", false );
>     
>     At least this is how kaddressbookmigrator does it [0]. I can't see any code to check whether the migrator should start when EnableAutostart is FALSE, so I assume KDE does this automagically when iterating through .desktop files in /usr/share/autostart.
>     
>     [0] http://quickgit.kde.org/index.php?p=kdepim-runtime.git&a=blob&f=migration%2Fkaddressbook%2Fkaddressbookmigrator.cpp
> 
> David Edmundson wrote:
>     The only place the logs are useful is the log viewer so we could do it on loading that.

I like the idea of /usr/share/autostart, but are you sure that if EnableAutostart=false will disable its startup magically, or is it checked somewhere?
Anyway I'd set some version number instead, so that in the future, if we need to migrate settings or similar, we'll be able to do it just by bumping the version number.

Moreover I'd go for a very small executable that just checks the setting, and eventually starts the real migrator and exits. In this way the executable can run every time, but stays very small and don't need to load libraries and the whole execution will be faster and will use less resources.


- Daniele Elmo


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107245/#review21605
-----------------------------------------------------------


On Nov. 7, 2012, 2:24 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107245/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2012, 2:24 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This patch adds a dialog to the ktp-kded module.
> 
> The dialog will run only once - it will check whether there are any Kopete logs that match your current KTp accounts and show you a list where you can select which logs you want to import.
> 
> After that it won't bother you never ever again.
> 
> For me the KDED module starts automatically on every login, but Martin told me that it's depends on some things and that it might not start every time. Please test this too.
> 
> I'd like to ask someone who actually speaks English to review the labels and messages. I suck at this...
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 024ad76 
>   logs-import-dialog.h PRE-CREATION 
>   logs-import-dialog.cpp PRE-CREATION 
>   telepathy-module.h 748437d 
>   telepathy-module.cpp 7ab13b2 
> 
> Diff: http://git.reviewboard.kde.org/r/107245/diff/
> 
> 
> Testing
> -------
> 
> Logged in, got dialog, succesfully imported all logs, logged out, logged in, no dialog.
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20121109/129fb282/attachment.html>