[Mesa-dev] Workflow Proposal

[Emma Anholt]

"

On Thu, Oct 7, 2021 at 1:38 AM Eero Tamminen <eero.t.tamminen at intel.com> wrote:
>
> Hi,
>
> On 6.10.2021 23.00, Jordan Justen wrote:
> > Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl> writes:
> >> On Wed, Oct 6, 2021 at 8:49 PM Jordan Justen <jordan.l.justen at intel.com> wrote:
> >>> I guess I missed where it was suggested that Marge should remove
> >>> Reviewed-by tags. I don't think Marge should ever remove something from
> >>> the commit message.
> >>
> >> AFAIU this is upstream Marge behavior. Once you enable the
> >> Approval->Rb tag conversion it removes existing Rb tags. Hence why we
> >> don't have the conversion enabled.
> >>
> >
> > Ah, I guess it is documented for --add-reviewers here:
> >
> > https://github.com/smarkets/marge-bot#adding-reviewed-by-tested-and-part-of-to-commit-messages
> >
> > "All existing Reviewed-by: trailers on commits in the branch will be
> >   stripped."
>
> This sounds horrible from the point of view of trying to track down
> somebody who knows about what's & why's of some old commit that is later
> on found to cause issues...
>
> For whom those extra Rb tags are a problem and why?

To explain Marge's behavior here: I think their concern is if it was
assigned to Marge with one set of reviewers, then failed to merge,
then assigned to Marge again with another set of reviewers flagged in
the MR, then they want to update the set of reviewers associated with
the merge without leaving in someone who had retracted their review.

For Mesa where people provide per-patch review, that's silly.  We
could fork and strip out that behavior, but in the original proposal
this flag wasn't getting enabled anyway so Marge's behavior is moot.

And, again, if you want to "track down somebody who knows about what's
& why's of some old commit", just click the link that's right there in
the commit, which gives you not just the names but also the comments
they had about the commit back when they reviewed it!