KDEREVIEW: share like connect and plasmate

Pino Toscano pino at kde.org
Wed Jan 2 21:38:46 UTC 2013


Hi,

apparently some people consider that all the issues of this review have 
been fixed, but really they were not.

Alle sabato 3 novembre 2012, Pino Toscano ha scritto:
> - PasswordAsker sounds like could be implemented on top of
> KPasswordDialog

Still there.

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

Still there.

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

Still there.

> - 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)

Still there.
 
> - 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

Still there, and it is even worse now: KStandardDirs is used to get a 
path for a _remote_ location.

> - 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)

Still there; the only change here was just using KLocale for the date 
output.

> -  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

Still there.

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

Still there.

> - why ImageLoader::run forces the formats?

Still there.

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

Now it writes the namespaces in a wrong way, closing the quoting 
manually and adding attributes by hand in a single string...

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

What about just finding the actions in the actionCollection() of the 
KTextEditor::View, and hiding them, instead of messing up with the 
XMLGUI document?

-- 
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/20130102/c939ebdc/attachment-0001.sig>


More information about the Plasma-devel mailing list