Merge lp://qastaging/~radumstoica/ubuntu/quantal/xfce4-terminal/scroll-alternate-src-patch into lp://qastaging/ubuntu/quantal/xfce4-terminal

Proposed by RaduStoica
Status: Merged
Merged at revision: 57
Proposed branch: lp://qastaging/~radumstoica/ubuntu/quantal/xfce4-terminal/scroll-alternate-src-patch
Merge into: lp://qastaging/ubuntu/quantal/xfce4-terminal
Diff against target: 456 lines (+93/-322)
6 files modified
.pc/applied-patches (+0/-1)
.pc/xubuntu_fix-invalid-anchor.patch/terminal/terminal-dialogs.c (+0/-313)
debian/changelog (+14/-0)
debian/patches/scroll-alternate-src-togglable.patch (+77/-0)
debian/patches/series (+1/-0)
terminal/terminal-dialogs.c (+1/-8)
To merge this branch: bzr merge lp://qastaging/~radumstoica/ubuntu/quantal/xfce4-terminal/scroll-alternate-src-patch
Reviewer Review Type Date Requested Status
Iain Lane Approve
David Henningsson (community) Needs Fixing
Ubuntu branches Pending
Review via email: mp+124570@code.qastaging.launchpad.net

Description of the change

Enabled scrolling in ncurses apps using mouse wheel. (lp: #947892)

To post a comment you must log in.
57. By RaduStoica

* debian/patches:
    - scroll-alternate-src-togglable.patch: enable scrolling in ncurses
      apps with mouse wheel. lp: #947892

Revision history for this message
David Henningsson (diwic) wrote :

Hi RaduStoica, and thanks for trying to help out!

I gave your merge proposal a quick review as part of the patch pilot program. And I noticed something strange while doing so - your commit does not only add the patch but also applies it, which means changes to files outside the debian directory.

Looking a bit back in history, it seems you're not the only one who has done this. As such I think it would make the most sense to do two commits: the first would just unapply all patches (to correct the previous stuff), the second one would add your patch without applying it (i e, only changes to debian/changelog, debian/patches/series and debian/patches/scroll-alternate-src-togglable.patch).

The actual patch content looks okay to me so hopefully it can be merged (by somebody with the appropriate upload rights, which I don't have) after that.

review: Needs Fixing
58. By RaduStoica

Unapplied all quilt patches.

Revision history for this message
RaduStoica (radumstoica) wrote :

> Hi RaduStoica, and thanks for trying to help out!
>
> I gave your merge proposal a quick review as part of the patch pilot program.
> And I noticed something strange while doing so - your commit does not only add
> the patch but also applies it, which means changes to files outside the debian
> directory.
>
> Looking a bit back in history, it seems you're not the only one who has done
> this. As such I think it would make the most sense to do two commits: the
> first would just unapply all patches (to correct the previous stuff), the
> second one would add your patch without applying it (i e, only changes to
> debian/changelog, debian/patches/series and debian/patches/scroll-alternate-
> src-togglable.patch).
>
> The actual patch content looks okay to me so hopefully it can be merged (by
> somebody with the appropriate upload rights, which I don't have) after that.

Thanks for taking the time to look at my branch! I'm still a bit confused about
how the quilt patching works, so that's why I commited the files with the patch
applied.

I've done a new commit now, with all patches unapplied. The patch for scrolling
in ncurses apps is still added in the debian/patches/series file, so I don't
think a second commit is needed. Let me know what's your opinion - I can make
a clean branch and add the patch to that one, if this one seems too messy.

Revision history for this message
Iain Lane (laney) wrote :

Thank you, I've merged it and uploaded to Quantal. There was no need to create a new version for the .pc fiddling - the mantra is one version per upload. Anyway, I'm sure I messed that up again while merging. It's complex :P

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

to all changes: