[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwin
Subject:    Re: Patch for bug 174769
From:       "Lucas Murray" <lmurray () undefinedfire ! com>
Date:       2008-11-18 4:26:24
Message-ID: f09827650811172026j16f4525dsaf5778dee10ba531 () mail ! gmail ! com
[Download RAW message or body]

- Patches should always be attached to the E-mail uncompressed
(Compressed if max attachment filesize is reached) for logging
purposes. I personally prefer the web-based approach though so using
both is best. =)
- Although I've set up a Git repo for KWin actually using it for
inter-developer branching is a pain due to Git-SVN. It's there only
for local commits, when the code is complete patches only please.
- Whitespace changes are usually done in a separate commit so the one
that actually changes code is easier to read.
- Line 440-442 of the diff, why did you change that? Now if you make a
change on either of those two tabs then switch to a different one then
the settings will not be saved. Please change it back to an
unconditional else.
- I got way too far many hunk fails to apply the patch so I could not
test it, please update your branch to the master and resend the patch.

On Tue, Nov 18, 2008 at 9:45 AM, David Nadlinger
<david.nadlinger@gmail.com> wrote:
> I have finally managed to finish a patch for bug 174769 – "Writing of
> settings to kwinrc should be postponed until user confirms desktop
> effects settings" (http://bugs.kde.org/show_bug.cgi?id=174769), which
> needs some reviews. You can find it at http://gist.github.com/26001.
> Better late then never, I guess.
>
> I hope that my changes don't break anything, because the KWin config
> system seems pretty fragile to me and lots of things are currently
> done in pretty inscrutable ways.
>
> There are a number of basically unneeded changes in the patch because
> I tried like three different approaches to the problem. However, I
> think they make the code easier to read. If you want to remove them,
> please feel free to do so, but I really have no time for this at the
> moment.
>
> The patch also contains quite a few purely cosmetical changes to
> kcmkwin/kwincompositing/main.cpp in order to comply to Lubos' cofing
> style standards. You can obtain a patch without them using the git
> tree at http://github.com/klickverbot/kwin/tree/compositingcrash (just
> clone the branches master and compositingcrash, rebase --interactive
> the history of compositingcrash to remove the questionable commit, and
> diff to master).
>
> I don't expect that I'll be in IRC very much the next few days, so if
> you have any further questions, please contact me via mail/this list.
>
>
> P.S.: If this made its way into trunk, this would be my first commit
> to KDE. Yay.
> _______________________________________________
> kwin mailing list
> kwin@kde.org
> https://mail.kde.org/mailman/listinfo/kwin
>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin

[prev in list] [next in list] [prev in thread] [next in thread]