reviewboard repository
Hi there, I was trying to post a patch for kparts today, but failed to find a kparts repository in reviewboard. So, what is the current way of submitting patch to any of frameworks libs? Cheers Michal ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: What are the plans with CamelCase includes?
On 24/12/13 07:38, Friedrich W. H. Kossebau wrote: Hi, what are the plans with offering CamelCase includes in KF5 (e.g. #include KLocale)? On a quick search could not find anything mentioned somewhere. If I saw correctly, then currently CamelCase includes (for existing classes) are only available by KF5::KDE4Support, due to being the module which has those files and also installs them. Also the KF5/KDE seems somehow (not yet found out how) only added to the include dirs by listing that module in target_link_libraries. Is that also the long-term plan? (I hope not, because I like the CamelCase includes, and other people who use Qt with CamelCase includes, as offered, and want to use some KF5 modules would welcome to have them, too). Would be willing to help out with this. There were plans to auto-generate them. I'm not sure what happened with that. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: reviewboard repository
On 24/12/13 09:57, Michal Humpula wrote: Hi there, I was trying to post a patch for kparts today, but failed to find a kparts repository in reviewboard. So, what is the current way of submitting patch to any of frameworks libs? Patch attached to an email to this list. However, we're currently concentrating on getting the tech preview out the door (and it's Christmas) so you may find you don't get much response for now. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[KParts] patch: don't emit urlChanged, when url is the same
Hi there,
as suggested without fully functional reviewboard, here is the little patch
for discussion.
Emit urlChanged only when actually changing url trough setUrl. The drawback of
this is that now the fast assignment is replaced by possibly expensive string
comparison. On the other hand, when the urls are the same, the emit wont fire
and whole bunch of potentially expensive operations connected to that signal
will not happen.
IMHO the same could/should be done for others setters.
Cheers
Michaldiff --git a/src/part.cpp b/src/part.cpp
index a485900..7fbea3d 100644
--- a/src/part.cpp
+++ b/src/part.cpp
@@ -475,8 +475,11 @@ void ReadOnlyPart::setUrl(const QUrl url)
{
Q_D(ReadOnlyPart);
-d-m_url = url;
-emit urlChanged(url);
+if (d-m_url != url)
+{
+ d-m_url = url;
+ emit urlChanged(url);
+}
}
QString ReadOnlyPart::localFilePath() const
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [KParts] patch: don't emit urlChanged, when url is the same
On Tuesday 24 December 2013 12:30:56 Michal Humpula wrote: Hi there, as suggested without fully functional reviewboard, here is the little patch for discussion. Emit urlChanged only when actually changing url trough setUrl. The drawback of this is that now the fast assignment is replaced by possibly expensive string comparison. That's ok, this method is typically not called 1 times per second. On the other hand, when the urls are the same, the emit wont fire and whole bunch of potentially expensive operations connected to that signal will not happen. Yep. The patch makes a lot of sense, please commit it - at least to frameworks. Do you also need it in kdelibs master? IMHO the same could/should be done for others setters. Which are they? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE, in particular KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: What are the plans with CamelCase includes?
On Tue, Dec 24, 2013 at 12:14 PM, Alex Merry k...@randomguy3.me.uk wrote: On 24/12/13 07:38, Friedrich W. H. Kossebau wrote: Hi, what are the plans with offering CamelCase includes in KF5 (e.g. #include KLocale)? On a quick search could not find anything mentioned somewhere. If I saw correctly, then currently CamelCase includes (for existing classes) are only available by KF5::KDE4Support, due to being the module which has those files and also installs them. Also the KF5/KDE seems somehow (not yet found out how) only added to the include dirs by listing that module in target_link_libraries. Is that also the long-term plan? (I hope not, because I like the CamelCase includes, and other people who use Qt with CamelCase includes, as offered, and want to use some KF5 modules would welcome to have them, too). Would be willing to help out with this. There were plans to auto-generate them. I'm not sure what happened with that. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel We need to use ecm_generate_headers on every module. I'll try to take some time after the 26th to do it. If somebody wants to do it meanwhile, please do and I'll review it. Aleix ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: What are the plans with CamelCase includes?
Hello,
On Tue, Dec 24, 2013 at 5:59 PM, Aleix Pol aleix...@kde.org wrote:
I'll try to take some time after the 26th to do it. If somebody wants to do
it meanwhile, please do and I'll review it.
I have some spare time and want to do it.. If I want to check one
example for it.. the code in KParts repo is good one? That's what I
will need?
include(ECMGenerateHeaders)
ecm_generate_headers(
Part Plugin PartManager MainWindow Event
BrowserExtension HistoryProvider BrowserInterface
BrowserRun StatusBarExtension BrowserOpenOrSaveQuestion
ScriptableExtension TextExtension HtmlExtension FileInfoExtension
ListingExtension
)
install(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/KParts DESTINATION
${INCLUDE_INSTALL_DIR} COMPONENT Devel )
Thanks!
--
Bhushan Shah
http://bhush9.github.io
IRC Nick : bshah on Freenode
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [KParts] patch: don't emit urlChanged, when url is the same
On Tuesday 24 December 2013 13:50:29 Michal Humpula wrote:
On Tuesday 24 of December 2013 13:22:05 David Faure wrote:
On Tuesday 24 December 2013 12:30:56 Michal Humpula wrote:
Hi there,
as suggested without fully functional reviewboard, here is the little
patch
for discussion.
Emit urlChanged only when actually changing url trough setUrl. The
drawback
of this is that now the fast assignment is replaced by possibly
expensive
string comparison.
That's ok, this method is typically not called 1 times per second.
On the other hand, when the urls are the same, the emit
wont fire and whole bunch of potentially expensive operations connected
to
that signal will not happen.
Yep. The patch makes a lot of sense, please commit it - at least to
frameworks. Do you also need it in kdelibs master?
Nope, no need. Just to be sure, pushing to master branch of
g...@git.kde.org:kparts
Yes.
Please fix the coding style first: the '{' belongs on the same line as the
if().
The code in frameworks/* should be very consistent now, since I reformatted it
all with astyle. So no excuse anymore for inconsistent coding style in new
code :)
But, if I have a green light, then I would like to change the second
callsite of urlChanged() as well. Please, see attached patch. The reasoning
is the same as with setUrl.
Coding style: please use { ... } even for one-line statements.
Looks OK otherwise.
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: [KParts] patch: don't emit urlChanged, when url is the same
On Tuesday 24 of December 2013 14:05:36 David Faure wrote:
On Tuesday 24 December 2013 13:50:29 Michal Humpula wrote:
On Tuesday 24 of December 2013 13:22:05 David Faure wrote:
On Tuesday 24 December 2013 12:30:56 Michal Humpula wrote:
Hi there,
as suggested without fully functional reviewboard, here is the little
patch
for discussion.
Emit urlChanged only when actually changing url trough setUrl. The
drawback
of this is that now the fast assignment is replaced by possibly
expensive
string comparison.
That's ok, this method is typically not called 1 times per second.
On the other hand, when the urls are the same, the emit
wont fire and whole bunch of potentially expensive operations
connected
to
that signal will not happen.
Yep. The patch makes a lot of sense, please commit it - at least to
frameworks. Do you also need it in kdelibs master?
Nope, no need. Just to be sure, pushing to master branch of
g...@git.kde.org:kparts
Yes.
Please fix the coding style first: the '{' belongs on the same line as the
if().
The code in frameworks/* should be very consistent now, since I reformatted
it all with astyle. So no excuse anymore for inconsistent coding style in
new code :)
But, if I have a green light, then I would like to change the second
callsite of urlChanged() as well. Please, see attached patch. The
reasoning
is the same as with setUrl.
Coding style: please use { ... } even for one-line statements.
Looks OK otherwise.
It's there. Thanks!
Cheers
Michal
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
XDG_APPS_INSTALL_DIR and KStandardDirs
This got a bit lost in the other thread on the kde4support tests... I want to advocate setting XDG_APPS_INSTALL_DIR to be $CMAKE_INSTALL_PREFIX/share/applications instead of $CMAKE_INSTALL_PREFIX/share/applications/kde5. I think we should be putting our application desktop files directory in share/applications, not in share/applications/kde5. The reason this has come up is that KStandardDirs currently contains a hack for how it compiles the value of xdgdata-apps. The upshot of this is that if the directory $CMAKE_INSTALL_PREFIX/share/applications does not exist, KStandardDirs::resourceDirs(xdgdata-apps) does not contain that directory, but instead contains XDG_APPS_INSTALL_DIR, which is $CMAKE_INSTALL_PREFIX/share/applications/kde5. In normal circumstances this does not happen (because the directory normally does exist, and if it doesn't it's kind of irrelevant), but it does cause one of the unit test to fail on Jenkins. Changing XDG_APPS_INSTALL_DIR would fix it, but it is not the only way to fix it. I just think that XDG_APPS_INSTALL_DIR *should* have a different value. Alex ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
