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 <cwabbo...@gmail.com>
I agree with the message, and the fixed up text by Connor is:
Reviewed-by: Eric Engestrom <eric.engest...@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 <apinhe...@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@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
