Review Request: Rename krunner plugin konsolesessions to konsoleprofiles

Aaron J. Seigo aseigo at kde.org
Mon Jan 7 21:01:32 UTC 2013



> On Jan. 7, 2013, 10:49 a.m., Aaron J. Seigo wrote:
> > i can understand changing the visible name, but why change the plugin name and the rest of it? that's an implementation detail, and while the name used may be technically "wrong" it is both a smaller change as well as a safer change to leave them as-is. each kconfig update script is something that needs to be processed at runtime, which must be present to provide an upgrade path, etc.
> > 
> > so imo only the user visible strings should be altered and the reset should remain as-is.
> 
> Jekyll Wu wrote:
>     I can see your concern of upgrading (especially for those who compile and install KDE manually instead of using package manager (like emerge) to ensure clean removal of previously installed files). Maybe one dummy konsolesessions.desktop (only containing Hidden=True) should also be installed to avoid users seeing both "konsole profiles" and "konsole sessions" in krunner.
>     
>     But the reason I want to also change the reset is not for users, but for developers/contributors, especially for those (including myself) who are not familiar with the history of those runners and just want to investigate and fix some bug quickly from time to time. 
>     
>     If I just change the user visible name, then there will be a konsoleseesions/ subfolder providing krunner_konsolesessions.so and konsolesessions.desktop, yet its visual name is "konsole profiles" , and its entry within krunnerrc is "konsolesessionsEnabled", and what it does is "konsole profiles".  That is just confusing(even more than now) to random contributors. Unless someone digs the code, commit history and this review request, he/she can hardly understand why he/she is seeing such a strange combination.  That even does not take into account all those names styled after "sessions" instead of "profiles" within the existing code.
>

moving the directory in the source tree from konsolesessions to konsole is fine for me. the rest should not be touched. the rare case of developer confusion on a rarely touched component, which can be resolved with a few minutes effort, does not come even close to the cost of making changes that affect every user install out there.


- Aaron J.


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


On Dec. 31, 2012, 6:58 p.m., Jekyll Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107881/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2012, 6:58 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Konsole currently does not provide the session feature like in kate and konqueror. That plugin works with konsole profiles, not the not-reimplemented-yet konsole sessions. 
> 
> Also, the equivalent plasmoid is properly named as "konsole profiles". 
> 
> 
> Diffs
> -----
> 
>   runners/CMakeLists.txt bb4b491 
>   runners/konsoleprofiles/CMakeLists.txt PRE-CREATION 
>   runners/konsoleprofiles/Messages.sh PRE-CREATION 
>   runners/konsoleprofiles/konsoleprofiles.cpp PRE-CREATION 
>   runners/konsoleprofiles/konsoleprofiles.desktop PRE-CREATION 
>   runners/konsoleprofiles/konsoleprofiles.h PRE-CREATION 
>   runners/konsoleprofiles/konsolesessions_renamed_to_konsoleprofiles.upd PRE-CREATION 
>   runners/konsolesessions/CMakeLists.txt c1d5cea 
>   runners/konsolesessions/Messages.sh 5f03904 
>   runners/konsolesessions/konsolesessions.cpp ed7550a 
>   runners/konsolesessions/konsolesessions.desktop 7d9ce5a 
>   runners/konsolesessions/konsolesessions.h a98c253 
> 
> Diff: http://git.reviewboard.kde.org/r/107881/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jekyll Wu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130107/b901f978/attachment.html>


More information about the Plasma-devel mailing list