Code review comment for lp:~stefan.goetz-deactivatedaccount/hipl/hidb-db

Hi,

On Mon, Aug 8, 2011 at 5:00 PM, Diego Biurrun <email address hidden> wrote:
> review needs-fixing
>
> On Fri, May 20, 2011 at 09:26:05PM +0000, Stefan Götz wrote:
>> Stefan Götz has proposed merging lp:~stefan.goetz/hipl/hidb-db into lp:hipl.
>>
>> HIP_DEBUG("sadb flushed\n");
>
> This debug line is rather pointless IMO, I'd remove it.
>
>> -out_err:
>> - return err;
>> + return 0;
>> }
>
> Why not make the function void instead of always returning the same value?
>
> There are more similar cases in this merge request, we need to come to
> a conclusion what to do with these cases.

well, shouldn't a merge review focus on the changes to be merged more than
on general problems with the code around it? Your points are valid but if I
see it right those parts of the code weren't touched by Stefan and in my
opinion unrelated to the goal of this branch.

There's the boyscout rule but you can only apply it so far else you never get
done with what you wanted to do in the first place or you needlessly bloat
the merge request, no?