Merge lp://staging/~unity-team/qtmir/build_with_clang into lp://staging/qtmir

Proposed by Michał Sawicz
Status: Merged
Approved by: Gerry Boland
Approved revision: 390
Merged at revision: 413
Proposed branch: lp://staging/~unity-team/qtmir/build_with_clang
Merge into: lp://staging/qtmir
Prerequisite: lp://staging/~unity-team/qtmir/liveCaption
Diff against target: 134 lines (+12/-12)
8 files modified
src/modules/Unity/Application/desktopfilereader.cpp (+2/-1)
src/modules/Unity/Application/mirsurfaceitem.h (+1/-1)
src/modules/Unity/Application/objectlistmodel.h (+1/-1)
src/platforms/mirserver/qmirserver.cpp (+1/-1)
src/platforms/mirserver/qmirserver_p.h (+2/-1)
tests/modules/common/fake_mirsurface.h (+1/-3)
tests/modules/common/qtmir_test.h (+1/-1)
tests/modules/common/stub_scene_surface.h (+3/-3)
To merge this branch: bzr merge lp://staging/~unity-team/qtmir/build_with_clang
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Alan Griffiths Pending
Review via email: mp+273115@code.staging.launchpad.net

This proposal supersedes a proposal from 2015-09-30.

Commit message

Build with clang (tests/gmock fails and is unfixable on our side i'd say)

Description of the change

* Are there any related MPs required for this MP to build/function as expected?
No, but https://code.launchpad.net/~aacid/mir/fix_forward_declarations/+merge/272751 is needed to get qtmir to actually build with clang :D

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

- argv[argc] = '\0';
+ argv[argc] = "\0";

This changes the meaning to be incorrect. It should be:

    argv[argc] = nullptr;

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

-struct QMirServerPrivate
+class QMirServerPrivate

It looks like a struct to me.

review: Abstain
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

> -struct QMirServerPrivate
> +class QMirServerPrivate
>
> It looks like a struct to me.

but it's used in a
  Q_DECLARE_PRIVATE(QMirServer)
that forward declares it as a class.

Revision history for this message
Gerry Boland (gerboland) wrote : Posted in a previous version of this proposal

Test build shows all CLang errors are due to:
1. gtest/gmock
2. Qt private headers
3. Mir (for which a fix has landed upstream)
Thus I'm happy.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Gerry Boland (gerboland) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches