24 May
2010
24 May
'10
12:43 a.m.
On Sun, May 23, 2010 at 2:20 PM, Allan McRae <allan@archlinux.org> wrote:
On 23/05/10 20:58, Andres P wrote:
Signed-off-by: Andres P<aepd87@gmail.com> ---
-1 from me. The new function does two things that are completely unrelated apart from the regex they use. It makes no sense to combine them. Also the function name does not reflect what it does at all.
There is a limit to how much code refactoring is useful. It needs to be balanced by readability.
There is a double combination in that patch. IMO one of them is fine, the one that combines install and changelog in a for loop. And that combination could be done in the two places, but keeping them separate.