|
From: | Jeff Abrahamson |
Subject: | Re: [RP] ratpoison, patches, and the future |
Date: | Tue, 30 Dec 2014 22:57:33 +0100 |
On Tue, Dec 30, 2014 at 12:15, Jeff Abrahamson wrote:
> - He expressed a concern that I developed all of these on the same
> branch. Actually, I did (and do) all of my work on separate branches, but
> in frustration that the code wasn't being merged into HEAD, I did it myself
> in my repository to make it easier to use the code, both for me and for
> others to try.
I understand what you were trying to do, but it's not ideal for
merging. Because right now it's either merge all or nothing (well, those
are the simplest options at least, you could always do more complicated
things to merge only parts). Also, the separate feature branches are
nice for people looking at the code because then all the changes are
logically grouped per feature. In order to keep it easy for people to
try out your version of ratpoison (with all the new stuff) you can still
rebase everything to upstream ratpoison (separate branch), and have your
master contain all the feature branches. This way you keep the
individual feature branches for inspection and merging with
upstream. Atlassian has a good explanation on how to maintain nice clean
feature branches [1].
> - He proposes that focus_policy become focus or focuspolicy (I prefer
> the latter, other opinions?). He also proposes that the third option be
> follows rather than ffm, I have no objection.
I also prefer focuspolicy, it is more descriptive and focus already
exists in ratpoison. It's a command not a variable, so it could
theoretically work but I feel like that might be confusing. Follow is a
nice improvement I think. Abbreviations tend to be confusing because
most people won't know that it stands for focus follows mouse. It might
also be nice to add a short explanation to the man page of the
difference between sloppy and follows. I know you've documented it in
the TeX, but I feel like like the vast majority of users won't even know
that that exists.
> - He's not so keen on my refactoring of cmd_select() and
> set_active_window_body(), suggesting they don't bring real improvement. I
> disagree. Both functions were overly long to my eye and harder to
> understand for it. (A quick git annotate on window.c even suggests that
> Jérémie may be the author of the FIXME on set_active_window_body(). ;-)
I'm not familiar enough with the code to really say anything about this
but I did take a quick look at the set_active_window_body change you
made (ca95ed9c). I'm not sure that extracting a function like you did
really fixes up the function as the FIXME intended. We could discuss the
benefits of this change and whether or not it improves readibility in a
significant way in depth but I feel like your other work/plans are much
more interesting and exciting things to talk about.
[Prev in Thread] | Current Thread | [Next in Thread] |