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 < > [email protected]> 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 > [email protected] > https://lists.blender.org/mailman/listinfo/bf-committers _______________________________________________ Bf-committers mailing list [email protected] https://lists.blender.org/mailman/listinfo/bf-committers
