<div>Is the patch good to commit? I'd like to commit before it gets stale given the changes taking place in desktop.h/cpp.</div>
<div>Chris<br><br></div>
<div class="gmail_quote">On Dec 10, 2007 11:17 PM, Christopher Blauvelt <<a href="mailto:cblauvelt@gmail.com">cblauvelt@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">
<div>
<div></div>
<div class="Wj3C7c"><br><br>
<div class="gmail_quote">On Dec 9, 2007 1:06 PM, Aaron J. Seigo <<a href="mailto:aseigo@kde.org" target="_blank">aseigo@kde.org</a>> wrote:<br>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0pt 0pt 0pt 0.8ex; BORDER-LEFT: rgb(204,204,204) 1px solid">
<div>On Sunday 09 December 2007, you wrote:<br>> I sent a patch on Thursday and hadn't heard anything back yet so I was just<br>> checking to see if you'd gotten a chance to take a look at it. Just <br><br>
</div>breifly, yes. please send patches to <a href="mailto:panel-devel@kde.org" target="_blank">panel-devel@kde.org</a> though, not directly<br>to me. i may be busy for a few days, it can get lost in my inbox (which is<br>
much busier and less focussed than the folders mailing lists get sorted out <br>into), etc.<br><br>as for the patch ... please follow the plasma coding style, which is the<br>kdelibs coding style[1]. in particular, there must be a space between if,
<br>foreach, while, etc... and the openning parens. so this: <br><br> if(!desktop) {<br><br>becomes<br><br> if (!desktop) {<br><br>if you're wondering what the point of that is, it actually is useful not only<br>for visual scanning but also for grepping of method calls (which don't have
<br>that space).<br><br>the for loop in newItems could be a foreach as well ...<br><br>the init() method will only work if it is run at desktop startup, due to the<br>config not having the url until the Url applet is saved; so if an icon is
<br>created at runtime and its config checked right away, it'll come up as not<br>having a url in the config. this could be adjust in the Url applet by having<br>it set the value in its config() in Url::setUrl if it is an issue.
<br><br>disconnectin signals is done for you on destruction, so strictly speaking you<br>don't need to do that in ~IconLoader.<br><br>the correct way to get the home dir is KGlobalSettings::desktopPath(), rather<br>than KUrl homedir(i18n("~/Desktop"));
<br><br>m_icons should not be called from constraintsUpdated without a guard since<br>that gets called repeatedly over the life of the containment. you'll need to<br>put a guard either in IconLoader::init or in<br>DefaultDesktop::constraintsUpdated to ensure that it doesn't get run more
<br>than once.<br><br>[1] <a href="http://techbase.kde.org/Policies/Kdelibs_Coding_Style" target="_blank">http://techbase.kde.org/Policies/Kdelibs_Coding_Style</a><br><font color="#888888"><br>--<br>Aaron J. Seigo<br>humru othro a kohnu se
<br>GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43<br><br>KDE core developer sponsored by Trolltech<br></font></blockquote></div><br></div></div>Made changes and it now reacts to a launcher applets being destroyed by the user without crashing when the corresponding file is deleted.
<br></blockquote></div><br>