Compiz

Merge lp://staging/~ivenvd/compiz/compiz.fix_1193792 into lp://staging/compiz/0.9.10

  1. compiz.fix_1193792
  2. Merge into 0.9.10
Proposed by Iven Hsu
Status: Merged
Approved by: MC Return
Approved revision: 3745
Merged at revision: 3745
Proposed branch: lp://staging/~ivenvd/compiz/compiz.fix_1193792
Merge into: lp://staging/compiz/0.9.10
Diff against target: 148 lines (+122/-0)
3 files modified
kde/window-decorator-kde4/kdecorationbridge.h (+115/-0)
kde/window-decorator-kde4/window.cpp (+6/-0)
kde/window-decorator-kde4/window.h (+1/-0)
To merge this branch: bzr merge lp://staging/~ivenvd/compiz/compiz.fix_1193792
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Sam Spilsbury Approve
MC Return Approve
Review via email: mp+170975@code.staging.launchpad.net

Commit message

KWD: Fix compile errors with KDE 4.11.

The KWin developers made kdecorationbridge.h private.

See: http://lists.freedesktop.org/archives/compiz/2013-March/003479.html

(LP: #1193792)

Description of the change

KWD: Fix compile errors with KDE 4.11. (LP: #1193792)

The KWin developers made kdecorationbridge.h private.

See: http://lists.freedesktop.org/archives/compiz/2013-March/003479.html

To post a comment you must log in.
Revision history for this message
MC Return (mc-return) wrote :

Iven, thanks a lot for that. +1
Could you please "Set (a) commit message" also ?

I guess you've tested this fix ?

review: Needs Information
Revision history for this message
MC Return (mc-return) wrote :

Here:

25 + virtual QuickTileMode quickTileMode(void) const;

a whitespace between quickTileMode and ( is missing.

The style of kdecorationbridge.h is not really what Compiz uses, but I guess this is okay as
"35 +This file is part of the KDE project."... ;)

Revision history for this message
Iven Hsu (ivenvd) wrote :

Yes, I've tested this.

What do you mean by commit message? I've already written a bzr commit message, I think.

Revision history for this message
MC Return (mc-return) wrote :

> Yes, I've tested this.
>
Cool.

> What do you mean by commit message? I've already written a bzr commit message,
> I think.

Probably you did, but the main commit message for bzr has to be additionally added to the merge request, otherwise the merge will fail.
It is the green "+" button above, which says "Set commit message".

Revision history for this message
Iven Hsu (ivenvd) wrote :

> Probably you did, but the main commit message for bzr has to be additionally
> added to the merge request, otherwise the merge will fail.
> It is the green "+" button above, which says "Set commit message".

Thanks. I'm not so familiar with launchpad. So if I want to resubmit the proposal, should I push to a new branch?

Revision history for this message
MC Return (mc-return) wrote :

>
> Thanks. I'm not so familiar with launchpad. So if I want to resubmit the
> proposal, should I push to a new branch?

No, that was already perfect. You do not need to push a new branch.

If you want to update your branch (with the whitespace fix I posted above, for example), simply make your changes locally, then "bzr commit" and then "bzr push" again. This will update this branch with your new commit.

See examples of this here:
https://code.launchpad.net/~mc-return/compiz/compiz.merge-thumbnail-improvements/+merge/170548

Workflow is like this:
First propose a branch, then get a review with improvement suggestions, implement and commit those, push again to update the merge request, ...

(If you have additional questions, just ask)

lp://staging/~ivenvd/compiz/compiz.fix_1193792 updated
3744. By Iven Hsu

Fix the coding-style issue with whitespace.

Revision history for this message
MC Return (mc-return) wrote :

Perfect. +1
Approve from me, but waiting for Sam to give this the final green light...
FYI: AFAIK trunk is frozen currently anyway, so do not worry if this might take a few days until it lands...

review: Approve
Revision history for this message
Sam Spilsbury (smspillaz) :
review: Approve
lp://staging/~ivenvd/compiz/compiz.fix_1193792 updated
3745. By Iven Hsu

Move kdecorationbridge.h to the right place.

Revision history for this message
Iven Hsu (ivenvd) wrote :

Sorry, my fault. Mistakely added the header file to wrong folder. Should be fixed now.

Revision history for this message
MC Return (mc-return) wrote :

So you've tested this now and made sure you can run KDE 4.11 with Compiz as WM ?

Revision history for this message
Iven Hsu (ivenvd) wrote :

> So you've tested this now and made sure you can run KDE 4.11 with Compiz as WM
> ?

Yes.

Revision history for this message
MC Return (mc-return) wrote :

> > So you've tested this now and made sure you can run KDE 4.11 with Compiz as
> WM
> > ?
>
> Yes.

Approved :)

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

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