[FusionDev] Code review

[Danny Baumann]

Hi,

> > - We do code review only where needed. I see no point in commenting most
> > patches, but I do see a point in commenting patches I have issues with /
> > questions about.
> 
> I assume you mean we comment the code review only where needed, as we
> have to review it all to know if there is anything to comment.

Yes, obviously.

> I was making the assumption that if we do review all patches, there
> will be something to comment for most of them. I just want to make
> sure that when a patch is commented, even if it is negative, it
> doesn't demoralize the commiter. If everyone gets comments, it looks
> far better than if a new or less experienced developer gets far more
> comments to every patch than the more seasoned developers.

I see your point, but people commiting to fusion/* will be somewhat
experienced anyway. Additionally, people should not be demoralized by
getting constructive (!) feedback, even if it contains a lot of
improvement suggestions.

> The goal is to make us sure that when we commit something, someone has
> read over it looking for anything at all. For tiny patches, commenting
> is redundant, but I am pretty sure that for any decent-sized patch,
> there is something. Even if it's just a bad comment for a function or
> a missing return check.
> 
> I do see your point here, I am just afraid that if we go into this
> thinking "should this be commented on?" instead of "What can I comment
> on here?", we'll get very few improvements. If we do ask "What can I
> comment here?" and the answer is nothing, then there is little point
> posting "OK" or similar.

I just don't want to see useless "I'm ok with that" and "Me too"
postings for every patch flooding the ML. If someone has a comment, even
if it is a sort-of-nitpicking-one, it's obviously fine to drop that on
the ML.

Regards,

Danny