[Mesa-dev] What does WIP really mean in an MR?

[Nanley Chery]

On Sat, Jun 29, 2019 at 4:40 PM Eric Engestrom <eric at engestrom.ch> wrote:
>
> On Saturday, 2019-06-29 22:59:21 +0200, apinheiro wrote:
> >
> > On 29/6/19 2:30, Rob Clark wrote:
> > > I had interpreted it as literally the "block the gitlab merge button"
> > > option, ie. "I want to get feedback but it is not ready to merge and
> > > I'll drop the WIP tag when I think it is"..
> >
> >
> > >
> > > (comments inline)
> > >
> > > On Fri, Jun 28, 2019 at 5:12 PM Ian Romanick <idr at freedesktop.org> wrote:
> > > > After a conversation yesterday with a couple of the other Intel devs,
> > > > I've come to the conclusion that *everyone* interprets WIP to mean
> > > > something different.  I heard no less than four interpretations.
> > > >
> > > > * This series is good.  It hasn't been reviewed, so don't click "merge."
> > > isn't that the point of a MR.. doesn't seem like a reason for "WIP"
> >
> >
> > I agree with Rob here. What I understand of WIP is that there is some reason
> > that prevents a MR to be merged, even if I still want it to be reviewed, or
> > if it is fully reviewed. For example, I send a MR with patches that I think
> > that are correct, so people can start to review it. But on a rebase, I found
> > that the branch causes regressions on piglit/cts/whatever. I still think
> > that the patches are correct, but those regressions need to be investigated
> > before pushing. So I just add a WIP on the MR to prevent to be merged by
> > mistake.
>
> Kind of just saying "+1" here, but yeah that's also my understanding of "WIP".
>

A help page in our gitlab instance [1] suggests a more relaxed/broader usage
of the WIP tag. So, I had been using WIP to block accidental merges that don't
meet the usual merge criteria (e.g., the code isn't ready or it lacks
reviewed-by tags).
I don't mind avoiding it on MRs that are gated only on the review tags.

1. https://gitlab.freedesktop.org/help/user/project/merge_requests/work_in_progress_merge_requests.md

> >
> > >
> > > > * This series has some sketchy bits.  It probably isn't ready for review
> > > > unless you've been tagged for design feedback.
> > > I guess I'd also use WIP for "I want some early feedback, but it isn't
> > > ready yet".. but in this case I'd also poke people who I wanted to
> > > look at it
> >
> >
> > I thought that in this case RFC was used. Or RFC was dropped on gitlab MRs?
>
> Agreed; and "RFC" is definitely being used:
> https://gitlab.freedesktop.org/mesa/mesa/merge_requests?state=all&search=RFC
>
> Adding "WIP" as well prevents accidental merging, so it makes sense.
>
> >
> >
> > >
> > > > * This series has been reviewed.  Incorporation of detailed feedback is
> > > > in progress, but it's going to take some time.
> > > I suppose also a case for "WIP"..
> > >
> > > > * This series is good, but there are some questionable patches at the end.
> > > I guess in this case, I'd reform things into multiple MR's, one with
> > > the parts ready to go, and one w/ the remaining WIP bits
> > >
> > > BR,
> > > -R
> > >
> > > > Due to this lack of common understanding, we discovered at least one MR
> > > > that was ready to go but had been ignored for months. :(  This makes me
> > > > wonder if other MRs have similarly languished for no good reason.
> > > >
> > > > Can we formulate some guidelines for how people should apply WIP to
> > > > their MRs and how people should interpret WIP when they see it on an MR?
>
> Once we reach a consensus, we should write it down in docs/submittingpatches.html

Agreed.

-Nanley

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev