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

[Campbell Barton]

Hi, there are a few cases this document doesn't cover
(understandably as they're not so typical).


How to best make updates to a patch?
------------------------------------

While proposing changes and having the patch author apply them is
almost always preferred, there are times when it is less trouble to
make the changes to the code directly.

Options include:

- Make a patch on top of their patch, submit the raw diff as a paste.
(it can be awkward to manually apply patches).

- Create a temporary branch and reference that, the patch author needs
to manually checkout and apply the patch.
(it can be awkward to download temporary branches, we need to remember
to delete them afterwards).

- Commandeer the patch and update it, allowing time for the original
author and others to review the changes.

Note that these situations are times when I'd often prefer to
collaborate on a branch to prepare a change for final review.

However it's not always practical, especially if the developer doesn't
have commit access.

In this situation github/gitlab's model works has an advantage as I'm
able to submit a pull-request to their repository.

NOTE: Phabricator already has support for proposing changes to a
single block of code, assuming this feature isn't practical (for
larger changes in multiple areas/files for example).


How to handle superficial updates to a patch?
---------------------------------------------

+1 for having diff's match the final commit.

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.

Or where patches are applied with known superficial issues that need
to have separate clean up commits applied afterwards (adding more
noisy commits).

An argument can be made that having patch authors get these kinds of
details right is something we should expect from them. Not something
to resolve on their behalf.

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.


When to commandeer a patch?
---------------------------

Occasionally there are older patches that can be reviewed and applied
with only minor changes even if those changes are to resolve merge
conflicts.

A one-size-fits-all policy for the situation is to give the original
author some *reasonable* amount of time to respond, only commandeering
the patch and updating if they don't reply or are unable to update the
patch.

In the case of opinionated/functional changes this makes sense. In the
case of updating the patch to resolve conflicts, this doesn't seem
necessary or useful.

On Tue, Jun 1, 2021 at 9:05 PM Sergey Sharybin via Bf-committers
<bf-committers at blender.org> wrote:
>
> Hi,
>
> Just a quick note. The bf-admin team worked on several process related
> documents to ensure a pleasant and efficient development process.
>
> Today we've updated wiki with the patch review process:
> https://wiki.blender.org/wiki/Process/Patch_Review
>
> Feedback is welcome.
>
> Best regards,
> - Sergey -
> --------------------------------------------------------------------
> Sergey Sharybin - sergey at blender.org - www.blender.org
> Principal Software Engineer, Blender
> Buikslotermeerplein 161, 1025 ET Amsterdam, the Netherlands
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> https://lists.blender.org/mailman/listinfo/bf-committers



-- 
- Campbell