<table><tr><td style="">davidedmundson added inline comments.</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D2345" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2345#inline-9420" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mart</span> wrote in <span style="color: #4b4d51; font-weight: bold;">shellcorona.cpp:469</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">how would you notice knf has been changed? only info about it is that key in kdeglobals</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">DBus signal from the KCM.</p>

<p style="padding: 0; margin: 8px;">It's how we do fonts, styles, colours etc.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2345#inline-9422" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mart</span> wrote in <span style="color: #4b4d51; font-weight: bold;">shellcorona.cpp:511</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">can we have kconf_update that are guarantee to run before plasmashell is started?</p>

<p style="padding: 0; margin: 8px;">was also thinking about just using the same file and have the config to be lost on switch (that's actually what the vdg proposed initially) tough seems really too dangerous to me</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p style="padding: 0; margin: 8px;">can we have kconf_update that are guarantee to run before plasmashell is started?</p></blockquote>

<p style="padding: 0; margin: 8px;">Good question.</p>

<p style="padding: 0; margin: 8px;">kconf_update also monitors for new scripts at runtime, which is another bug waiting to happen.</p>

<p style="padding: 0; margin: 8px;">We might need to have some sort of:</p>

<p style="padding: 0; margin: 8px;">if (exists(plasma-shell-appletsrc) {</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">move(plasma-shell-appletsrc   , plasma-shell-lnf-appletsrc)</pre></div>

<p style="padding: 0; margin: 8px;">}</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D2345#inline-9421" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mart</span> wrote in <span style="color: #4b4d51; font-weight: bold;">shellcorona.cpp:720</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">i'm thinking about adding a<br />
QObject *Applet::graphicsRepresentation() as the access to it seems to have became necessary in many places</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Might be nice, but that doesn't solve the thing that's the problem.</p>

<p style="padding: 0; margin: 8px;">You can't just randomly delete things owned by some other object. Knowing which objects owns what is a core part of good design.</p>

<p style="padding: 0; margin: 8px;">From what I can see:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">Applet owns the DeclarativeAppletScript (member var script)</li>
<li class="remarkup-list-item">DeclarativeAppletScript owns the AppletInterface (AppletScript set as parent)</li>
</ul>

<p style="padding: 0; margin: 8px;">delete the applet (which is literally the next line) and you delete this anyway.</p>

<p style="padding: 0; margin: 8px;">If this solves a problem, you're solving it in the wrong place.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMAWORKSPACE Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2345" rel="noreferrer">https://phabricator.kde.org/D2345</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>mart, Plasma, davidedmundson<br /><strong>Cc: </strong>davidedmundson, ivan, plasma-devel, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>