On Sun, May 23, 2010 at 2:20 PM, Allan McRae <[email protected]> wrote: > On 23/05/10 20:58, Andres P wrote: >> >> Signed-off-by: Andres P<[email protected]> >> --- > > -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.
