Re: Review Request for KWayland for inclusion in frameworks

2016-05-07 Thread Michael Pyne
On Sat, May 7, 2016 13:50:05 David Faure wrote:
> Hi guys,
> 
> Can you check the CI for KWayland? ASAN detects an invalid memory usage
> in KWayland::Client::OutputDevice::Private::doneCallback() called after
> KWayland::Client::OutputDevice::~OutputDevice().
> 
> https://build.kde.org/view/Frameworks%20kf5-qt5/job/kwayland%20master%20kf5-> 
> qt5/28/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/kwa
> yland_testWaylandOutputDevice/

I find that report kind of confusing. The line 323 of 
test_wayland_outputdevice.cpp is just a QSignalSpy constructor, it shouldn't 
involve destruction of an OutputDevice (what ASAN is warning about here). I 
wonder if it maybe has something to do with the modern signal/slot connection 
syntax in use?

For what it's worth there's a relevant Coverity entry (CID 1340943), noting 
that the TestWaylandOutputDevice class does not have an initializer for 
m_queue within the constructor (or in any of the other class members it looked 
at). But I don't see how that would be related either.

Regards,
 - Michael Pyne
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request for KWayland for inclusion in frameworks

2016-05-07 Thread David Faure
Hi guys,

Can you check the CI for KWayland? ASAN detects an invalid memory usage
in KWayland::Client::OutputDevice::Private::doneCallback() called after 
KWayland::Client::OutputDevice::~OutputDevice().

https://build.kde.org/view/Frameworks%20kf5-qt5/job/kwayland%20master%20kf5-qt5/28/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/kwayland_testWaylandOutputDevice/

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

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


Re: Review Request for KWayland for inclusion in frameworks

2016-05-02 Thread Martin Graesslin
Hi David,

did you forget to update the version number of KWayland to 5.22 or is 
something still missing in KWayland?

Cheers
Martin

On Sunday, April 17, 2016 10:42:08 AM CEST David Faure wrote:
> Hi Martin,
> 
> Very nice run down of the check list!
> 
> I approve the move of kwayland to frameworks, please proceed
> (preferrably earlier than in two weeks, since that would be close to release
> date).
> 
> Cheers,
> David.
> 
> On Friday 15 April 2016 10:08:57 Martin Graesslin wrote:
> > Hi frameworks-developers,
> > 
> > based on the thread "KWayland as framework" [1] I want to start the review
> > process for including KWayland into frameworks.
> > 
> > To make it easier I'm giving an overview:
> > Policies [2]:
> > * Tier 1/integration - dependencies on Qt::Gui (5.4) and Wayland (1.7)
> > * directory structure
> > 
> >  ** name of toplevel directory: kwayland
> >  ** README.md: check
> >  ** COPYING.LIB: check
> >  ** metadata.yaml: check
> >  ** docs subdirectory: no additional documentation
> >  ** src with dedicated subdirectories: check, there is src/client and src/
> > 
> > server
> > 
> >  ** examples subdirectory: no dedicated examples
> >  ** autotests subdirectory: check
> >  ** tests subdirectory: check
> >  ** cmake subdirectory: not needed
> > 
> > * automatic unit test: the current line coverage for client library is 86
> > %, the line coverage for server library is 80 %
> > * binary compatibility: KWayland has been ABI stable since it got added to
> > Plasma
> > * documentation: yes, the API is documented
> > * localization: not needed, no user visible strings
> > 
> > * buildsystem:
> >   ** KF5WaylandConfig.cmake is generated
> >   ** KF5WaylandConfigVersion.cmake is generated
> >   ** no other cmake files are installed
> >   ** library names are camel case: KF5WaylandServer and KF5WaylandClient
> >   ** dependencies to other frameworks: it's tier 1, doesn't apply
> > 
> > * commits are reviewed: yes, but KWayland is already using phabricator
> > * CI failures are treated as stop the line events: I like my view green
> > ;-)
> > 
> > * compiler requirements:
> >   ** gcc 4.5: I don't know, the minimum gcc version my distribution offers
> >   is
> > 
> > 4.7
> > 
> >   ** clang 3.1: I don't know, the minimum clang version my distribution
> >   offers> 
> > is 3.5
> > 
> >  **  VS2012: doesn't apply, it's Linux only [5]
> >  ** constraints for other C++11 features: it compiles on build.kde.org, so
> > 
> > should be fine
> > 
> > Guidelines [3]:
> > * Policies: see above
> > * translations: doesn't apply
> > * split repository: yes, all commits were extracted when creating the
> > project * astyle: the code follows the kdelibs coding style from the
> > start, didn't run astyle again
> > * Dependencies on deprecated API: none
> > * module in frameworks: not yet, currently still in Plasma [4]
> > * kde-build-metadata: not yet, currently still in Plasma [4]
> > * CI Jobs: currently still for Plasma [4]:
> > https://build.kde.org/job/kwayland %20master%20kf5-qt5/
> > * bugs.kde.org project: currently still in Plasma [4]
> > * reviewboard: yes
> > * README.md: yes
> > 
> > If you have any further questions regarding the move, please ask. If I
> > don't get any blocking feedback in the next two weeks, I'm going to
> > request the move and hope to get it into next frameworks release if that
> > works with David ;-)
> > 
> > Thanks,
> > Martin
> > 
> > [1]
> > https://mail.kde.org/pipermail/kde-frameworks-devel/2016-March/032116.htm
> > l [2] https://community.kde.org/Frameworks/Policies
> > [3] https://community.kde.org/Frameworks/CreationGuidelines
> > [4] I'll do those tasks if we get the green light for move to frameworks,
> > otherwise it'll stay in Plasma
> > [5] some code uses linux includes, to support other Unix-systems porting
> > is
> > needed



signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request for KWayland for inclusion in frameworks

2016-04-17 Thread David Faure
Hi Martin,

Very nice run down of the check list!

I approve the move of kwayland to frameworks, please proceed
(preferrably earlier than in two weeks, since that would be close to release 
date).

Cheers,
David.


On Friday 15 April 2016 10:08:57 Martin Graesslin wrote:
> Hi frameworks-developers,
> 
> based on the thread "KWayland as framework" [1] I want to start the review 
> process for including KWayland into frameworks.
> 
> To make it easier I'm giving an overview:
> Policies [2]:
> * Tier 1/integration - dependencies on Qt::Gui (5.4) and Wayland (1.7)
> * directory structure
>  ** name of toplevel directory: kwayland
>  ** README.md: check
>  ** COPYING.LIB: check
>  ** metadata.yaml: check
>  ** docs subdirectory: no additional documentation
>  ** src with dedicated subdirectories: check, there is src/client and src/
> server
>  ** examples subdirectory: no dedicated examples
>  ** autotests subdirectory: check
>  ** tests subdirectory: check
>  ** cmake subdirectory: not needed
> * automatic unit test: the current line coverage for client library is 86 %, 
> the line coverage for server library is 80 %
> * binary compatibility: KWayland has been ABI stable since it got added to 
> Plasma
> * documentation: yes, the API is documented
> * localization: not needed, no user visible strings
> * buildsystem:
>   ** KF5WaylandConfig.cmake is generated
>   ** KF5WaylandConfigVersion.cmake is generated
>   ** no other cmake files are installed
>   ** library names are camel case: KF5WaylandServer and KF5WaylandClient
>   ** dependencies to other frameworks: it's tier 1, doesn't apply
> * commits are reviewed: yes, but KWayland is already using phabricator
> * CI failures are treated as stop the line events: I like my view green ;-)
> * compiler requirements:
>   ** gcc 4.5: I don't know, the minimum gcc version my distribution offers is 
> 4.7
>   ** clang 3.1: I don't know, the minimum clang version my distribution 
> offers 
> is 3.5
>  **  VS2012: doesn't apply, it's Linux only [5]
>  ** constraints for other C++11 features: it compiles on build.kde.org, so 
> should be fine
> 
> Guidelines [3]:
> * Policies: see above
> * translations: doesn't apply
> * split repository: yes, all commits were extracted when creating the project
> * astyle: the code follows the kdelibs coding style from the start, didn't 
> run 
> astyle again
> * Dependencies on deprecated API: none
> * module in frameworks: not yet, currently still in Plasma [4]
> * kde-build-metadata: not yet, currently still in Plasma [4]
> * CI Jobs: currently still for Plasma [4]: https://build.kde.org/job/kwayland
> %20master%20kf5-qt5/
> * bugs.kde.org project: currently still in Plasma [4]
> * reviewboard: yes
> * README.md: yes
> 
> If you have any further questions regarding the move, please ask. If I don't 
> get any blocking feedback in the next two weeks, I'm going to request the 
> move 
> and hope to get it into next frameworks release if that works with David ;-)
> 
> Thanks,
> Martin
> 
> [1] https://mail.kde.org/pipermail/kde-frameworks-devel/2016-March/032116.html
> [2] https://community.kde.org/Frameworks/Policies
> [3] https://community.kde.org/Frameworks/CreationGuidelines
> [4] I'll do those tasks if we get the green light for move to frameworks, 
> otherwise it'll stay in Plasma
> [5] some code uses linux includes, to support other Unix-systems porting is 
> needed

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5

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