Alle giovedì 3 gennaio 2013, Giorgos Tsiapaliokas ha scritto: > On 2 January 2013 23:38, Pino Toscano <p...@kde.org> wrote: > > > - 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. > > *[1] The 2 above are some good impovements for the future, but not > something that should keep plasmate in playground.
Giving a fixed size to a dialog, and putting its widgets in a fixed place causes issues with: a) different fonts than the ones the dialog was designed b) texts of different size, like translated ones so no, this is not an "improvement", this is a *bug*. > > > - 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. > > In which ui files are you referring to? plasmate/publisher/publisher.ui plasmate/publisher/remoteinstaller/remoteinstaller.ui plasmate/editors/kconfigxt/kconfigxteditor.ui plasmate/startpage.ui (Not that there are many .ui files in plasmate, so you could have even checked on your own.) > > - 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) > > > > > > > > > > > > - 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 > > > > Still there. > > *[1] the above 3 are good improvements but not something which should > keep plasmate in playground. They are issues which don't fit extragear either, and IMHO they are more close to playground-quality code. > > > - 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... > > When I was implementing this one I couldn't find some API in > QXmlStreamWriter > which does the job and I assume that the same applies for rest of the > people who > read my review, am I missing something? QXmlStreamWriter::writeNamespace could be a guess. > > - 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? > > this is the only documented solution in > techbase<http://techbase.kde.org/Development/Architecture/KDE4/XMLGUI > _Technology>, so I don't see any reason > to avoid it and its also the recommended one. That's point #3, while point #2 is similar to what I suggested. > P.S.: [1] As it regards issues like those, we can always disagree > about stuff like that > but the point is to focus on the major issues. There were many "major issues" back when I did my first reviews, and some of them still are present; you did and still are underestimating the importance/impact of the issues I reported. -- Pino Toscano
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel