Mir

Merge lp://qastaging/~vanvugt/mir/resize-BufferStream into lp://qastaging/mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1200
Proposed branch: lp://qastaging/~vanvugt/mir/resize-BufferStream
Merge into: lp://qastaging/mir
Diff against target: 182 lines (+98/-0)
8 files modified
include/server/mir/compositor/buffer_stream.h (+1/-0)
include/test/mir_test_doubles/mock_buffer_stream.h (+1/-0)
include/test/mir_test_doubles/stub_buffer_stream.h (+4/-0)
src/server/compositor/buffer_stream_surfaces.cpp (+5/-0)
src/server/compositor/buffer_stream_surfaces.h (+1/-0)
tests/integration-tests/compositor/test_buffer_stream.cpp (+73/-0)
tests/integration-tests/test_swapinterval.cpp (+1/-0)
tests/unit-tests/compositor/test_buffer_stream.cpp (+12/-0)
To merge this branch: bzr merge lp://qastaging/~vanvugt/mir/resize-BufferStream
Reviewer Review Type Date Requested Status
Kevin DuBois (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Alan Griffiths Needs Fixing
Review via email: mp+193555@code.qastaging.launchpad.net

Commit message

Add resize() support to BufferStream, in preparation for resizable surfaces.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good functionality-wise.

8 + virtual void resize(const geometry::Size &size)

According to our style guide, the preferred position for placing 'const' is after the type, although we don't require it. However, we do require the '&'/'*' to be adjacent to the type.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I know the style guide and have raised the issue several times since April:
https://lists.ubuntu.com/archives/mir-devel/2013-April/000098.html

I will, as always, use the more sensible style where it makes sense in isolation. When modifying code adjacent to the older style however, I will keep the older style.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I know the style guide and have raised the issue several times since April:
> https://lists.ubuntu.com/archives/mir-devel/2013-April/000098.html
>
> I will, as always, use the more sensible style where it makes sense in
> isolation. When modifying code adjacent to the older style however, I will
> keep the older style.

The adjacent code does not follow this style, so that rationale doesn't apply.

Please use the more sensible style described in the style guideline.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Alan, I think you're in the minority industry-wise if you describe the style guide's approach as sensible. But I know I'm the minority within mir-team, so will compromise.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

looks good to me

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The last remaining "Needs Fixing" was fixed yesterday. And there are two other human approvals aside from that, so landing...

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