[Bf-committers] Simple steps to get an harmonious collaboration

Sebastian Parborg darkdefende at gmail.com
Fri Jun 4 12:17:09 CEST 2021


I think most of these issues with be solved when we have CI bots?

In most bigger open source projects I contribute to, the bots makes sure
that everything is formated correctly and that there are no new
warning/errors in the code.

If there are any issues, they will post a comment pointing the issue out
and telling the patch author to fix it.

When there is no more issues the bot adds a tag to the review stating
that it has passed the tests and is ready for a human to review.

This cuts down considerably on time spent both by the reviewer and the
patch author.

On Fri, Jun 04, 2021 at 11:50:42AM +0200, Sybren A. Stüvel via Bf-committers wrote:
> On Wed, 2 Jun 2021 at 07:56, Campbell Barton via Bf-committers <
> bf-committers at blender.org> wrote:
> 
> > If the proposed policies are followed to the letter, we end up in
> > situations where extra review iterations would be required for running
> > clang-format, stripping what-space and spelling mistakes.
> >
> 
> I don't really see a problem here. Somebody needs to do that work anyway.
> This is typically done by the patch author if they have commit access, and
> a reviewer otherwise. As someone with too many unread emails from
> still-to-be-reviewed patches, I would prefer to not have to do these
> cleanups myself when I land a patch. Often such formatting & whitespace
> issues can be solved permanently by configuring the patch author's IDE to
> do this for them, after which all subsequent patches will be fine in this
> respect. To me that looks like a better solution than to accept
> badly-formatted patches. I know I'm polarizing the situation here, but I
> also feel that if one extra review iteration is so troublesome, we could
> also look at the source of that trouble and see if we can improve things.
> For example, is `arc diff` too hard to use, and should we maybe look at
> some other tool to make reviewing easier? Or should we have an automatic
> cleanup on patches, so that they adhere to the coding standard?
> 
> 
> > When the issues are trivial though (trailing white-space) it adds some
> > burden on the patch reviewer to have to remember that some patch which
> > was otherwise fine - needs an extra iteration.
> >
> 
> You don't have to remember when you write it in the review comments.
> 
> Sybren
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> https://lists.blender.org/mailman/listinfo/bf-committers


More information about the Bf-committers mailing list