KDEREVIEW: share like connect and plasmate

Ben Cooksley bcooksley at kde.org
Wed Jan 2 21:41:41 GMT 2013


On Thu, Jan 3, 2013 at 10:38 AM, Pino Toscano <pino at kde.org> wrote:
> 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?

Due to this review issue, Plasmate has been returned to KDE Review. My
query regarding Share-Like-Connect still stands as well.

>
> --
> Pino Toscano
>

Regards,
Ben

> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel at kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>




More information about the kde-core-devel mailing list