Re: KScreen/libkscreen commit message policies
On Mon, Nov 11, 2019 at 10:58 PM Luigi Toscano wrote: > > Roman Gilg ha scritto: > > Hi, > > > > I just pushed commit message policies to KScreen [1] and libkscreen > > [2] (linked below from GitHub so they can be read with Markdown > > formatted). > > > > I intend to revert in the future all manual commits to KScreen and > > libkscreen deliberately ignoring the policies. Here "manual" means > > here that I won't revert commits based on a script that are regularly > > pushed to many repositories (like release number changes) or > > automatically named merge commits. > > > > If you have questions or other comments you can also reach me on IRC > > #plasma, romangg. > > Does it also mean that any typo fix in user visible strings should go through > review requests? I would classify such changes in general as "trivial changes" in the sense that they are obvious and do not need to be discussed or reviewed. If in rare cases this does not hold, the change can get reverted and need to go through review instead. But I assume this to be a rare occurrence. Note though that in any case the commit message guideline must be respected. Calling such a commit for example "fix(kcm): spell X correctly" should be enough. A question long term is if there should be a separate type for i18n-only changes (for example "i18n(kcm): spell X correctly"). What's your opinion on that? This could help down the line setting up automatic tooling (as said only long term relevant). Cheers, Roman > > -- > Luigi
D24433: Move URL parsing methods from kioslave to query object
bruns added a subscriber: broulik.
bruns added inline comments.
INLINE COMMENTS
> kio_search.cpp:88
> -return jsonQueryForType(QStringLiteral("Document"));
> -} else if (path.endsWith(QLatin1String("/images"))) {
> -return jsonQueryForType(QStringLiteral("Image"));
@broulik - why `endsWith()`?
REPOSITORY
R293 Baloo
BRANCH
extend_query_url
REVISION DETAIL
https://phabricator.kde.org/D24433
To: iasensio, #baloo, meven, ngraham, astippich
Cc: broulik, bruns, kde-frameworks-devel, #dolphin, hurikhan77, lots0logs,
LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich,
spoorun, ngraham, abrahams
D25267: Improve XCF support
sandsmark updated this revision to Diff 69618. sandsmark added a comment. Fixed masks as well, now even some more complex XCF files work again. REPOSITORY R287 KImageFormats CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25267?vs=69617=69618 REVISION DETAIL https://phabricator.kde.org/D25267 AFFECTED FILES src/imageformats/gimp_p.h src/imageformats/xcf.cpp To: sandsmark, aacid, cfeck, apol, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25267: Improve XCF support
sandsmark updated this revision to Diff 69617. sandsmark added a comment. Forgot to fix the layer offset reading, and read the precision after instead of before the image properties REPOSITORY R287 KImageFormats CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25267?vs=69616=69617 REVISION DETAIL https://phabricator.kde.org/D25267 AFFECTED FILES src/imageformats/gimp_p.h src/imageformats/xcf.cpp To: sandsmark, aacid, cfeck, apol, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25267: Improve XCF support
sandsmark created this revision. sandsmark added reviewers: aacid, cfeck, apol, vkrause. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. sandsmark requested review of this revision. REVISION SUMMARY Now it can at least handle the simplest XCF files I have. Biggest difference seems to be that they changed to 64bit for offsets from version 11 and upwards. Still some issues with more complex layers (aka. not completely plain images), but this is better than nothing. TEST PLAN Tested with some simple XCFs I had laying around that didn't get thumbnails earlier. REPOSITORY R287 KImageFormats REVISION DETAIL https://phabricator.kde.org/D25267 AFFECTED FILES src/imageformats/gimp_p.h src/imageformats/xcf.cpp To: sandsmark, aacid, cfeck, apol, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25265: Support adding a contact to a specific backend
jbbgameich added a reviewer: Plasma: Mobile. REPOSITORY R307 KPeople REVISION DETAIL https://phabricator.kde.org/D25265 To: jbbgameich, apol, #plasma:_mobile Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
Re: KScreen/libkscreen commit message policies
El dilluns, 11 de novembre de 2019, a les 23:25:34 CET, Albert Astals Cid va escriure: > El dilluns, 11 de novembre de 2019, a les 22:41:18 CET, Roman Gilg va > escriure: > > Hi, > > > > I just pushed commit message policies to KScreen [1] and libkscreen > > [2] (linked below from GitHub so they can be read with Markdown > > formatted). > > That's not how it works, you can't override the manifesto that says we can > all push everywhere :) I think i kind of did a fool of myself by commenting before reading the article. I can't really find stuff openly against the manifesto on it. Sorry for being a bad community member :/ Cheers, Albert > > Cheers, > Albert > > > > > I intend to revert in the future all manual commits to KScreen and > > libkscreen deliberately ignoring the policies. Here "manual" means > > here that I won't revert commits based on a script that are regularly > > pushed to many repositories (like release number changes) or > > automatically named merge commits. > > > > If you have questions or other comments you can also reach me on IRC > > #plasma, romangg. > > > > Thanks, > > Roman > > > > [1] https://github.com/KDE/kscreen/blob/master/CONTRIBUTING.md > > [2] https://github.com/KDE/libkscreen/blob/master/CONTRIBUTING.md > > > > > > >
Re: KScreen/libkscreen commit message policies
El dilluns, 11 de novembre de 2019, a les 22:41:18 CET, Roman Gilg va escriure: > Hi, > > I just pushed commit message policies to KScreen [1] and libkscreen > [2] (linked below from GitHub so they can be read with Markdown > formatted). That's not how it works, you can't override the manifesto that says we can all push everywhere :) Cheers, Albert > > I intend to revert in the future all manual commits to KScreen and > libkscreen deliberately ignoring the policies. Here "manual" means > here that I won't revert commits based on a script that are regularly > pushed to many repositories (like release number changes) or > automatically named merge commits. > > If you have questions or other comments you can also reach me on IRC > #plasma, romangg. > > Thanks, > Roman > > [1] https://github.com/KDE/kscreen/blob/master/CONTRIBUTING.md > [2] https://github.com/KDE/libkscreen/blob/master/CONTRIBUTING.md >
Re: Bulk replacement of projects.kde.org on Frameworks modules
Elvis Angelaccio ha scritto: > > > On 11/11/19 22:52, Luigi Toscano wrote: >> Hi, >> basically all Frameworks components reference the ECM website >> using the old projects.kde.org URL, which is long gone and >> it is just a (partial) redirect. >> >> See for example: >> >> set_package_properties(ECM PROPERTIES TYPE REQUIRED >> DESCRIPTION "Extra CMake Modules." >> URL "https://projects.kde.org/projects/kdesupport/extra-cmake-modules;) >> >> Can I go around a bulk-replace all the URLs with >> https://commits.kde.org/extra-cmake-modules, so that it would look like: >> >> set_package_properties(ECM PROPERTIES TYPE REQUIRED >> DESCRIPTION "Extra CMake Modules." >> URL "https://commits.kde.org/extra-cmake-modules;) > > +1 > > Using commits.kde.org like this feels weird though (i.e. using it > without a commit hash). commits.kde.org is the general redirector, and it will continue to work even after the switch to gitlab. We have a bit of documentation here: https://techbase.kde.org/Projects/Documentation/KDE_(health_table)#Links_to_kde_repos Ciao -- Luigi
Re: Bulk replacement of projects.kde.org on Frameworks modules
On 11/11/19 22:52, Luigi Toscano wrote: > Hi, > basically all Frameworks components reference the ECM website > using the old projects.kde.org URL, which is long gone and > it is just a (partial) redirect. > > See for example: > > set_package_properties(ECM PROPERTIES TYPE REQUIRED > DESCRIPTION "Extra CMake Modules." > URL "https://projects.kde.org/projects/kdesupport/extra-cmake-modules;) > > Can I go around a bulk-replace all the URLs with > https://commits.kde.org/extra-cmake-modules, so that it would look like: > > set_package_properties(ECM PROPERTIES TYPE REQUIRED > DESCRIPTION "Extra CMake Modules." > URL "https://commits.kde.org/extra-cmake-modules;) +1 Using commits.kde.org like this feels weird though (i.e. using it without a commit hash). Something like https://code.kde.org/extra-cmake-modules would look much better imho. Just my 2c, maybe something that could be done after the move to gitlab. > > There are few additional URLs which use the old website and I would replace > them as well using the same pattern. > > I'm asking to avoid ~70 almost-identical review requests. > > Ciao > Ciao Elvis
Re: KScreen/libkscreen commit message policies
Roman Gilg ha scritto: > Hi, > > I just pushed commit message policies to KScreen [1] and libkscreen > [2] (linked below from GitHub so they can be read with Markdown > formatted). > > I intend to revert in the future all manual commits to KScreen and > libkscreen deliberately ignoring the policies. Here "manual" means > here that I won't revert commits based on a script that are regularly > pushed to many repositories (like release number changes) or > automatically named merge commits. > > If you have questions or other comments you can also reach me on IRC > #plasma, romangg. Does it also mean that any typo fix in user visible strings should go through review requests? -- Luigi
D24433: Move URL parsing methods from kioslave to query object
iasensio updated this revision to Diff 69613. iasensio marked 2 inline comments as done. iasensio added a comment. Simplify helper function REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24433?vs=67361=69613 BRANCH extend_query_url REVISION DETAIL https://phabricator.kde.org/D24433 AFFECTED FILES src/kioslaves/search/kio_search.cpp src/lib/query.cpp To: iasensio, #baloo, meven, ngraham, astippich Cc: bruns, kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, abrahams
Bulk replacement of projects.kde.org on Frameworks modules
Hi, basically all Frameworks components reference the ECM website using the old projects.kde.org URL, which is long gone and it is just a (partial) redirect. See for example: set_package_properties(ECM PROPERTIES TYPE REQUIRED DESCRIPTION "Extra CMake Modules." URL "https://projects.kde.org/projects/kdesupport/extra-cmake-modules;) Can I go around a bulk-replace all the URLs with https://commits.kde.org/extra-cmake-modules, so that it would look like: set_package_properties(ECM PROPERTIES TYPE REQUIRED DESCRIPTION "Extra CMake Modules." URL "https://commits.kde.org/extra-cmake-modules;) There are few additional URLs which use the old website and I would replace them as well using the same pattern. I'm asking to avoid ~70 almost-identical review requests. Ciao -- Luigi
KScreen/libkscreen commit message policies
Hi, I just pushed commit message policies to KScreen [1] and libkscreen [2] (linked below from GitHub so they can be read with Markdown formatted). I intend to revert in the future all manual commits to KScreen and libkscreen deliberately ignoring the policies. Here "manual" means here that I won't revert commits based on a script that are regularly pushed to many repositories (like release number changes) or automatically named merge commits. If you have questions or other comments you can also reach me on IRC #plasma, romangg. Thanks, Roman [1] https://github.com/KDE/kscreen/blob/master/CONTRIBUTING.md [2] https://github.com/KDE/libkscreen/blob/master/CONTRIBUTING.md
D25265: Support adding a contact to a specific backend
jbbgameich updated this revision to Diff 69608. jbbgameich added a comment. Use more future proof roleNames and Enum REPOSITORY R307 KPeople CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25265?vs=69603=69608 BRANCH master REVISION DETAIL https://phabricator.kde.org/D25265 AFFECTED FILES src/CMakeLists.txt src/datasourcemodel.cpp src/datasourcemodel.h src/declarative/peopleqmlplugin.cpp src/personpluginmanager.cpp src/personpluginmanager.h To: jbbgameich, apol Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25265: Support adding a contact to a specific backend
jbbgameich added a reviewer: apol. REPOSITORY R307 KPeople REVISION DETAIL https://phabricator.kde.org/D25265 To: jbbgameich, apol Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25265: Support adding a contact to a specific backend
jbbgameich created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. jbbgameich requested review of this revision. REVISION SUMMARY - Add overload of PersonPluginManager::addContact that takes a pluginId - Add a DataSourceModel that can later be extended with additional metadata on the backends. TEST PLAN - Functionality works in plasma-phonebook (https://invent.kde.org/jbbgameich/plasma-phonebook/commit/5e370222059fb093fdbfb99f52b87845c78bedf2) REPOSITORY R307 KPeople BRANCH master REVISION DETAIL https://phabricator.kde.org/D25265 AFFECTED FILES src/CMakeLists.txt src/datasourcemodel.cpp src/datasourcemodel.h src/declarative/peopleqmlplugin.cpp src/personpluginmanager.cpp src/personpluginmanager.h To: jbbgameich Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D23411: Fix crash in writer collection and cleanup
astippich added a comment. Thanks! I pushed a fix which disables the test if taglib is not installed REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D23411 To: astippich, bruns, mgallien, ngraham Cc: justinkb, apol, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D24686: Replace usage of deprecated SlaveBase::config() by SlaveBase::configValue
This revision was automatically updated to reflect the committed changes. Closed by commit R320:49440c0cbea1: Replace usage of deprecated SlaveBase::config() by SlaveBase::configValue (authored by meven). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24686?vs=68023=69592 REVISION DETAIL https://phabricator.kde.org/D24686 AFFECTED FILES CMakeLists.txt nfs/nfsv2.cpp nfs/nfsv3.cpp sftp/kio_sftp.cpp smb/kio_smb_dir.cpp To: meven, dfaure Cc: elvisangelaccio, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
D24433: Move URL parsing methods from kioslave to query object
bruns added inline comments.
INLINE COMMENTS
> query.cpp:317
> +{
> +const QString jsonQuery(QStringLiteral("{\"dayFilter\": 0,\
> + \"monthFilter\": 0, \
dayFilter/monthFilter/yearFilter are pointless, as they just default, but have
to be parsed in `fromJSON`.
> query.cpp:329
> +if (path == QLatin1String("/documents")) {
> +return jsonQueryForType(QStringLiteral("Document"));
> +} else if (path.endsWith(QLatin1String("/images"))) {
can be replaced by `QStringLiteral("{\"type\":[\"Document\"]}")`, dito below.
REPOSITORY
R293 Baloo
BRANCH
extend_query_url
REVISION DETAIL
https://phabricator.kde.org/D24433
To: iasensio, #baloo, meven, ngraham, astippich
Cc: bruns, kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, LeGast00n,
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, abrahams
D24433: Move URL parsing methods from kioslave to query object
meven accepted this revision. meven added a comment. This revision is now accepted and ready to land. Seems good to me. Tested and works REPOSITORY R293 Baloo BRANCH extend_query_url REVISION DETAIL https://phabricator.kde.org/D24433 To: iasensio, #baloo, meven, ngraham, astippich Cc: kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D24433: Move URL parsing methods from kioslave to query object
meven added a reviewer: astippich. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D24433 To: iasensio, #baloo, meven, ngraham, astippich Cc: kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D24433: Move URL parsing methods from kioslave to query object
meven added a reviewer: ngraham. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D24433 To: iasensio, #baloo, meven, ngraham Cc: kde-frameworks-devel, #dolphin, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D23411: Fix crash in writer collection and cleanup
justinkb added a comment. The test incorrectly fails when a user builds kfilemetadata without taglib REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D23411 To: astippich, bruns, mgallien, ngraham Cc: justinkb, apol, kde-frameworks-devel, #baloo, hurikhan77, lots0logs, LeGast00n, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
Re: Sysadmin Load Reduction: Subversion Infrastructure
Hi all, This has been quite the long thread for one that was started only around 2 days ago, so thought I would summarise things: On the topic of removal of the Subversion mirrors, there have been no objections, so we will proceed with this. With regards to removal of access for everyone but translators, there have been two objections noted to this. One concerned compliance with the Manifesto, whilst the other was concerned with creating obstacles. Given that restricting access to the websites is still compliant with the Manifesto despite the lack of a clear clause permitting this I don't believe that not granting access by default to Subversion should be an issue, so would like to proceed with this as well. That leaves the last point, which has generated the most contention, the removal of WebSVN (websvn.kde.org). Based on the responses, it would appear that translation workflows are currently heavily embedded with and tightly dependent on this service. This would indicate that we would need to keep this service around for now. With just a few websites left on Subversion though, I would like to know if the translation teams have a plan for how they will continue to do things in the long term, and whether this includes a migration away from Subversion (as otherwise we are maintaining indefinitely a Subversion repository, customised hooks, mirroring infrastructure, WebSVN, and support for the email notifications from those customised hooks within other systems such as our IRC Bots and Bugzilla) Regards, Ben
KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.13 - Build # 155 - Fixed!
BUILD SUCCESS Build URL https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.13/155/ Project: kf5-qt5 WindowsMSVCQt5.13 Date of build: Mon, 11 Nov 2019 10:02:01 + Build duration: 12 min and counting
Re: Sysadmin Load Reduction: Subversion Infrastructure
On Mon, Nov 11, 2019 at 3:58 AM Luigi Toscano wrote: > > Ben Cooksley ha scritto: > > On Sun, Nov 10, 2019 at 11:19 AM Albert Astals Cid wrote: > >> > >> El dissabte, 9 de novembre de 2019, a les 21:22:16 CET, Ben Cooksley va > >> escriure: > >>> On Sun, Nov 10, 2019 at 8:37 AM Albert Astals Cid wrote: > > El dissabte, 9 de novembre de 2019, a les 19:27:07 CET, Ben Cooksley va > escriure: > > On Sun, Nov 10, 2019 at 7:18 AM Alexander Potashev > > wrote: > >> > >> сб, 9 нояб. 2019 г. в 03:20, Ben Cooksley : > >>> This would include the shutdown of WebSVN in particular, which when > >>> coupled with the shutdown of our two CGit instances would also allow > >>> for us to eliminate an entire virtual machine from our systems. > >> > >> Will there be any web interface for SVN after shutdown of WebSVN? > >> > >> Can we assume https://phabricator.kde.org/source/svn/ remains > >> available during the next 10 years? > >> > > > > Phabricator's browser will be retired as part of the shutdown of > > Phabricator, which will take place once Gitlab has assumed > > responsibility for code hosting and review, and the tasks have been > > migrated from Phabricator. > > > > Should WebSVN be shutdown as well, then there would be no web > > interface to our SVN repository. > > That's not acceptable. > >>> > >>> Mind explaining why? > >> > >> Because we use it in l10n.kde.org to link to po files. > > > > Mind detailing what those links are used for? > > > >> > >>> Bear in mind that there is a cost both in terms of infrastructure, and > >>> people time to maintain a service such as WebSVN. > >> > >> We have money, we don't have to shut down things we use because there is a > >> cost. > > > > I wasn't referring to monetary cost there, I was referring to the flow > > on effects (such as having to maintain the necessary components on the > > master server to allow for the Subversion repository to be mirrored). > > > > Note also the "people time" component there. > > Sure, but please see my previous questions: > - can we extend the space of rosetta? It already has a partial checkout, and > 100 GB of free space (which can be kept down We would need to ask the system hosters to provide this, as Rosetta is a donated system (if memory serves, Rosetta is currently IOPS constrainted, so using it to host WebSVN may over-burden the system) > - if that's not enough, can we simply setup a machine which periodically sync > from the svn repository? You are probably going to tell me that it does not > work without server support, but from what I'm reading about svnsync, I don't > think it should overload the server if executed every 30 minutes. > Are we sure that we still need something on the master server? Let's try it > first. I'm afraid you cannot use svnsync with KDE's Subversion repository, that utility hasn't been able to handle it for many, many years now (it crashes if memory serves - [ade] hit this and documented it on his blog back around the time it broke which was before the switch to Git got underway) We have custom tooling which uses rsync instead to mirror the repository. Custom tooling is necessary because plain rsync cannot be used to reliably mirror the repository - because it has 1.5 million commits in it, which means over 3 million inodes on disk. Each of these would have to be checked by plain usage of rsync, which takes a substantial amount of time even on NVMe SSD storage - during which time the local mirror of the repository may be inconsistent (rendering it unusable), and which also generates a matching disk load on the master server, reducing it's performance. The RSync endpoint for the mirrors to contact is the component on the master server that has to be maintained. > - in case it's not clear, I'm volunteering to maintain that system. > > Ciao > -- > Luigi Cheers, Ben
D25249: KDirModel: port to qCDebug, with its own category
ervin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D25249 To: dfaure, mlaurent, sandsmark, ervin Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25079: [CopyJob] Increase the amount of data sendfile can copy at once
meven marked 3 inline comments as done. meven added inline comments. INLINE COMMENTS > file_unix.cpp:266 > #ifdef USE_SENDFILE > -bool use_sendfile = buff_src.st_size < 0x7FFF; > +bool use_sendfile = true; > #endif This prevented using sendfile for file bigger than 2 GB. Our code has no reason to have this restriction. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25079 To: meven, dfaure, #frameworks, davidedmundson Cc: ahmadsamir, sitter, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
D25079: [CopyJob] Increase the amount of data sendfile can copy at once
meven marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D25079 To: meven, dfaure, #frameworks, davidedmundson Cc: ahmadsamir, sitter, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
