Re: [Mesa-dev] [PATCH] docs: advice to resolve discussion on gitlab MR doc
On 16/5/19 13:35, Eric Engestrom wrote: 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 I agree with the message, and the fixed up text by Connor is: Reviewed-by: Eric Engestrom 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. I pushed the patch with Connor text plus this suggestion. Thanks for the quick review! (and for basically rewrite my broken grammar english text) On Thu, May 16, 2019 at 11:36 AM Alejandro Piñeiro 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.) Make changes and update your branch based on feedback + 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. Old, stale MR may be closed, but you can reopen it if you still want to pursue the changes You should periodically check to see if your MR needs to be -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: advice to resolve discussion on gitlab MR doc
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 I agree with the message, and the fixed up text by Connor is: Reviewed-by: Eric Engestrom 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 > 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.) > > > > > >Make changes and update your branch based on feedback > > + 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. > >Old, stale MR may be closed, but you can reopen it if you > > still want to pursue the changes > >You should periodically check to see if your MR needs to be > > -- > > 2.19.1 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] docs: advice to resolve discussion on gitlab MR doc
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 On Thu, May 16, 2019 at 11:36 AM Alejandro Piñeiro 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.) > > >Make changes and update your branch based on feedback > + 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. >Old, stale MR may be closed, but you can reopen it if you > still want to pursue the changes >You should periodically check to see if your MR needs to be > -- > 2.19.1 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] docs: advice to resolve discussion on gitlab MR doc
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.) Make changes and update your branch based on feedback + 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. Old, stale MR may be closed, but you can reopen it if you still want to pursue the changes You should periodically check to see if your MR needs to be -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
