[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: plasmashell never emits setStage('desktop');
From:       Marco Martin <notmart () gmail ! com>
Date:       2014-09-13 12:11:37
Message-ID: CAD6_BouB0=yyspcPsO3HRSeQyOWZm0OAJgQBhdXm59OSYdTH4A () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Sat, Sep 13, 2014 at 12:17 PM, =C3=80lex Fiestas <afiestas@kde.org> wrot=
e:
>
> > Signaling that the qml is ready and keeping track centrally that all
> > containments everywhere, and all their applets have indeed their qml
> ready
> > as well, has to pass trough quite long hoops, not much to be done aroun=
d
> > that.
> Well for a newcomer it is, lots of huge methods, Containment being an
> Applet,
> Applet having special cases for Containments lots of friend classes etc.
>

If is possible to simplify the code a bit without altering the behavior I'm
all for it (another reason a better test coverage is needed, so behavior
changes that introduce bugs could be avoided)

the reason for having such special cases and friend applets (a lot simpler
already than plasma1 tough) is to avoid exposing public api that would end
up being intended to be used only internally: that besides being ugly,
would make things harder to debug as well, since you wouldn't be sure
anymore that the behavior is caused by something appening inside the
library.. or some by external caller, that shouldn't use that method not
supposed to be used.


> I am not saying that all the logic inside is not needed, what I am saying
> is
> that for a newcomer trying to fix a bug the code is too complex and we
> might
> want to refactor it bit a bit if possible and when desirable.
>
> Anyway, will try to hunt the bug down!
>

It should now be fixed after my last push and my last couple of review
requests:D (p-w and p-f)
(note the changes in p-f have an accompainying test, my wet dream is to
have one for most of bug fixes)

--
Marco Martin

[Attachment #5 (text/html)]

<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Sep 13, 2014 \
at 12:17 PM, Àlex Fiestas <span dir="ltr">&lt;<a href="mailto:afiestas@kde.org" \
target="_blank">afiestas@kde.org</a>&gt;</span> wrote:<blockquote class="gmail_quote" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""> \
&gt; Signaling that the qml is ready and keeping track centrally that all<br> &gt; \
containments everywhere, and all their applets have indeed their qml ready<br> &gt; \
as well, has to pass trough quite long hoops, not much to be done around<br> &gt; \
that.<br> </span>Well for a newcomer it is, lots of huge methods, Containment being \
an Applet,<br> Applet having special cases for Containments lots of friend classes \
etc.<br></blockquote><div><br></div><div>If is possible to simplify the code a bit \
without altering the behavior I&#39;m all for it (another reason a better test \
coverage is needed, so behavior changes that introduce bugs could be \
avoided)</div><div><br></div><div>the reason for having such special cases and friend \
applets (a lot simpler already than plasma1 tough) is to avoid exposing public api \
that would end up being intended to be used only internally: that besides being ugly, \
would make things harder to debug as well, since you wouldn&#39;t be sure anymore \
that the behavior is caused by something appening inside the library.. or some by \
external caller, that shouldn&#39;t use that method not supposed to be \
used.</div><div>  </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> I am not saying that all the logic \
inside is not needed, what I am saying is<br> that for a newcomer trying to fix a bug \
the code is too complex and we might<br> want to refactor it bit a bit if possible \
and when desirable.<br> <br>
Anyway, will try to hunt the bug down!<br></blockquote><div><br></div><div>It should \
now be fixed after my last push and my last couple of review requests:D (p-w and \
p-f)</div><div>(note the changes in p-f have an accompainying test, my wet dream is \
to have one for most of bug fixes)</div></div><br></div><div \
class="gmail_extra">--</div><div class="gmail_extra">Marco Martin</div></div>



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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic