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

[apinheiro]

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 <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.


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 <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