[Mesa-dev] [PATCH] docs: advice to resolve discussion on gitlab MR doc

Eric Engestrom eric.engestrom at intel.com
Thu May 16 11:35:31 UTC 2019


On Thursday, 2019-05-16 13:14:46 +0200, Connor Abbott wrote:
> Some grammar nits:
> 
> - "Resolve Discussion" goes before "button" as it modifies it.
> - It's either "This way..." or "In this manner...", not "In this
> way...", although the latter is a little too stilted/over-formal here.
> - This isn't a hypothetical or another situation where "would know..."
> is appropriate.
> - "...which didn't" (since it's short for "which didn't get handled").
> 
> The corrected text is:
> 
> After an update, for the feedback you handled, close the feedback
> discussion with the "Resolve Discussion" button. This way the reviewer
> knows which feedback got handled and which didn't.
> 
> I definitely didn't notice this when I started using Gitlab either.
> With the fixed text:
> 
> Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>

I agree with the message, and the fixed up text by Connor is:
Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>

If this isn't too bikeshed-y, I would also say "the reviewers know" as
there are potentially (and usually) more than one, but this really
doesn't matter that much.

> 
> On Thu, May 16, 2019 at 11:36 AM Alejandro PiƱeiro <apinheiro at igalia.com> wrote:
> >
> > For newcomers to gitlab, it is not evident that it is better to press
> > the "Resolve Discussion" button when you update your branch handling
> > feedback.
> > ---
> >
> > As the commit message says, it is not always evident. I was pointed to
> > do that when I started to use gitlab, and just today I mentioned it to
> > two different people that didn't know about that.
> >
> > Having said so, I feel that the specific text needs some poulishing
> > first, so any suggestion is welcome.
> >
> >  docs/submittingpatches.html | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
> > index 020e73d09ec..147b97d76e1 100644
> > --- a/docs/submittingpatches.html
> > +++ b/docs/submittingpatches.html
> > @@ -258,6 +258,10 @@ your email administrator for this.)
> >  </p>
> >  <ul>
> >    <li>Make changes and update your branch based on feedback
> > +  <li>After an update, for the feedback you handled, close the
> > +  feedback discussion with the button "Resolve Discussion". In this
> > +  way the reviewer would know which feedback got handled and which
> > +  not.
> >    <li>Old, stale MR may be closed, but you can reopen it if you
> >      still want to pursue the changes
> >    <li>You should periodically check to see if your MR needs to be
> > --
> > 2.19.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list