Code review comment for lp://qastaging/~radumstoica/ubuntu/quantal/xfce4-terminal/scroll-alternate-src-patch

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.

« Back to merge proposal