[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: KDEREVIEW: share like connect and plasmate
From: Giorgos Tsiapaliokas <terietor () gmail ! com>
Date: 2013-01-02 23:09:23
Message-ID: CAODYyLavm7pY675r4Axj8se_XvoGyunmiwwOW15eP5Lhw0pwqQ () mail ! gmail ! com
[Download RAW message or body]
On 2 January 2013 23:38, Pino Toscano <pino@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.
>
> > - 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?
> - 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.
> - why ImageLoader::run forces the formats?
>
> Still there.
yes we missed that
> > - 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?
> - 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.
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.
--
Giorgos Tsiapaliokas (terietor)
KDE Developer
terietor.gr
[Attachment #3 (text/html)]
<div dir="ltr"><br><br><div class="gmail_quote">On 2 January 2013 23:38, Pino Toscano \
<span dir="ltr"><<a href="mailto:pino@kde.org" \
target="_blank">pino@kde.org</a>></span> wrote:<blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div \
class="im"> > - BranchDialog sounds like could be replaced with<br>
> KInputDialog::getText with a custom validator<br>
<br>
</div>Still there.<br>
<div class="im"><br>
> - CommitDialog, other than being a KDialog, should better be use<br>
> layouts instead of placing widgets manually<br>
<br>
</div>Still there.<br></blockquote><div><br></div><div><div>*[1] The 2 above are some \
good impovements for the future, but not something</div><div>that should keep \
plasmate in playground.</div></div><div> </div><blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> - a numer of .ui files sets bold/bigger texts, but using a qt rich<br>
> text which forces a font size (and in few cases also the font face)<br>
<br>
</div>Still there.</blockquote><div><br></div><div>In which ui files are you \
referring to?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
> - TimeLine::loadTimeLine does a funky job in putting translated bits<br>
> among the git output; a better way would be parsing the output<br>
> extracting the various details, and composing a new ad-hoc string<br>
> (and the date would need localization, as the FIXME say)</div></blockquote><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="im"> > - StartPage::saveNewProjectPreferences \
saves the status of all the<br> > js/py/etc radio buttons separately... saving the \
index or the name of<br> > the active one would be much \
easier</div></blockquote><div> </div><blockquote class="gmail_quote" style="margin:0 \
0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"></div><div \
class="im"> > - EditPage::showTreeContextMenu uses the internalPointer() of \
the<br> > model, which makes it prone to break if the model changes<br>
> implementation internally<br>
<br>
</div>Still there.</blockquote><div> </div><div>*[1] the above 3 are good \
improvements but not something which should</div><div>keep plasmate in playground. \
</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="im">
> - why ImageLoader::run forces the formats?<br>
<br>
</div>Still there.</blockquote><div><br></div><div>yes we missed that</div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="im"> > - why KConfigXtWriter writes \
<kcfg> prologue/epilogue by hand?<br> <br>
</div>Now it writes the namespaces in a wrong way, closing the quoting<br>
manually and adding attributes by hand in a single \
string...</blockquote><div><br></div><div>When I was implementing this one I \
couldn't find some API in QXmlStreamWriter</div><div>which does the job and I \
assume that the same applies for rest of the people who</div> <div>read my review, am \
I missing something?</div><div><br></div><div><br></div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div class="im"> > - TextEditor::modifyToolBar does a big \
no-no job in looking for<br> > actions (never ever compare to translated strings, \
especially when<br> > coming from other components)<br>
<br>
</div>What about just finding the actions in the actionCollection() of the<br>
KTextEditor::View, and hiding them, instead of messing up with the<br>
XMLGUI document?<br></blockquote></div><br>this is the only documented solution in <a \
href="http://techbase.kde.org/Development/Architecture/KDE4/XMLGUI_Technology">techbase</a>, \
so I don't see any reason<div>to avoid it and its also the recommended one.</div> \
<div><br><div>P.S.: [1] As it regards issues like those, we can always disagree \
about stuff like that</div><div>but the point is to focus on the major \
issues.</div><div><br>-- <br><div dir="ltr">Giorgos Tsiapaliokas (terietor)<br> KDE \
Developer<br><br><a href="http://terietor.gr" target="_blank">terietor.gr</a></div> \
</div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic