KDEREVIEW: share like connect and plasmate

[Pino Toscano]

Hi,

Alle giovedì 1 novembre 2012, Aaron J. Seigo ha scritto:
> This is to inform everyone that the plasmate and share-like-connect
> repositories have been moved into KDE Review so that, if all goes
> according to plan, we can move them to their more permanent homes in
> a couple of weeks.
> 
> Most of the apps in the plasmate repo have actually been in past SC
> releases; it's just the main app, plasmate, that is still fairly
> new.

Regarding plasmate: I fixed earlier a number of i18n issues (ugh...), 
and the layout of two .ui files.
On the other hand, there are still the following issues I found looking 
around:

- there are some dialogs being done as QDialog: what about converting 
them to KDialog, for a common look'n'feel with the rest of KDE apps, and 
a bit less layouting code?

- PasswordAsker sounds like could be implemented on top of 
KPasswordDialog

- BranchDialog sounds like could be replaced with KInputDialog::getText 
with a custom validator

- CommitDialog, other than being a KDialog, should better be use layouts 
instead of placing widgets manually

- a numer of .ui files sets bold/bigger texts, but using a qt rich text 
which forces a font size (and in few cases also the font face)

- metadata.ui has an error message whose color is forced as red; please 
use KMessageWidget or at least use KColorScheme to get the fore-/back-
ground colors for errors

- RemoteInstaller uses "/var/tmp/plasmaremoteinstaller/" as destination 
directory, which is a bit too generic (at least appending the user name 
and chmod'ing it 600 would help); also there is a race between the KIO 
exists and the mkdir calls

- TimeLine::loadTimeLine does a funky job in putting translated bits 
among the git output; a better way would be parsing the output 
extracting the various details, and composing a new ad-hoc string (and 
the date would need localization, as the FIXME say)

- main installs a message handler which makes MainWindow emit a 
signal... which is caught by itself: why not just put it in main.cpp, 
and in case there is a main window notify it to write to the konsole 
widget? also, such handler currently writes to 
/var/tmp/plasmatepreviewerlog.txt, which is not a good thing to do...

-  StartPage::saveNewProjectPreferences saves the status of all the 
js/py/etc radio buttons separately... saving the index or the name of 
the active one would be much easier

- EditPage::showTreeContextMenu uses the internalPointer() of the model, 
which makes it prone to break if the model changes implementation 
internally

- KonsolePreviewer (ab)uses /var/tmp/plasmatepreviewerlog.txt as 
temporary file name, please use KTemporaryFile

- wrt Publisher::doCMake:
  - should check that `cmake` exists (see KStandardDirs::findExe) prior
    to do all the work
  - $KDEDIRS is not set by default by KDE, but only by the user
  - check the return value of a command invocation, before starting the
    next command
  - when commands fail, instead of a generic failure error, what about
    showing the output+error log?

- SigningDialog::validateParams could use some already existing email 
validation method (iirc there is a basic one in kdelibs and a better one 
in kdepimlibs)

- why ImageLoader::run forces the formats?

- KConfigXtEditor writes/replaces XML by hand... is that really a good 
thing to do (think about proper escaping, etc)? consider just using 
QDom/QXml for the job

- why KConfigXtWriter writes <kcfg> prologue/epilogue by hand?

- TextEditor::modifyToolBar does a big no-no job in looking for actions 
(never ever compare to translated strings, especially when coming from 
other components)

I think that's enough for now.

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20121103/ec16e2bf/attachment.sig>