[PATCH 2/2] dim: Fix outlook_author variable.

[Lucas De Marchi]

On Thu, Feb 22, 2018 at 04:33:46PM +0100, Daniel Vetter wrote:
> On Wed, Feb 21, 2018 at 03:35:11PM -0800, Rodrigo Vivi wrote:
> > On Wed, Feb 21, 2018 at 03:12:22PM -0800, Lucas De Marchi wrote:
> > > On Wed, Feb 21, 2018 at 03:03:44PM -0800, Lucas De Marchi wrote:
> > > > On Wed, Feb 21, 2018 at 02:02:00PM -0800, Rodrigo Vivi wrote:
> > > > > On Wed, Feb 21, 2018 at 01:47:58PM -0800, Lucas De Marchi wrote:
> > > > > > On Mon, Feb 19, 2018 at 04:51:46AM +0100, Daniel Vetter wrote:
> > > > > > > On Thu, Feb 15, 2018 at 11:14:10AM -0800, Rodrigo Vivi wrote:
> > > > > > > > The current version author_outlook ends up like
> > > > > > > > "First, Last," instead of "First Last" as it should be.
> > > > > > > > 
> > > > > > > > Honestly I'm so glad that this never worked. So I had a chance
> > > > > > > > to contact authors before merging the patch and fix the
> > > > > > > > outlook + patchwork mess.
> > > > > > > > 
> > > > > > > > But anyways, if we decide to stay with this logic let's at least
> > > > > > > > fix it.
> > > > > > > > 
> > > > > > > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > > > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > > > > > ---
> > > > > > > >  dim | 2 +-
> > > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/dim b/dim
> > > > > > > > index 4493b48b1494..ed8b9b03c42a 100755
> > > > > > > > --- a/dim
> > > > > > > > +++ b/dim
> > > > > > > > @@ -732,7 +732,7 @@ function checkpatch_commit_push
> > > > > > > >  	author=$(git show -s $sha1 --format="format:%an")
> > > > > > > >  	committer=$(git show -s $sha1 --format="format:%cn")
> > > > > > > >  	# outlook mangles mails into "Last, First"
> > > > > > > > -	author_outlook=$(git show -s $sha1 --format="format:%an" | sed -e 's/\([^ ]*\) \(.*\)/\2, \1/')
> > > > > > > > +	author_outlook=$(git show -s $sha1 --format="format:%an" | sed -e 's/\([^ ]*\) \(.*\)/\2 \1/;s/,//')
> > > > > > > 
> > > > > > > The above regex (the original one) moves from "Last, First" to "First
> > > > > > > Last". 
> > > > > > 
> > > > > > It seems it's actually from "First Last" to "Last, First".... but this
> > > > > > is not what the comment says... it should be converting the other way
> > > > > > around.
> > > > > > 
> > > > > > > Sounds like we need a 2nd path for the other direction? Also
> > > > > > > simpler to remove the ',' with one regex, like this:
> > > > > > > 
> > > > > > > 
> > > > > > > 	author_from_outlook=$(git show -s $sha1 --format="format:%an" | sed -e 's/\([^ ]*\), \(.*\)/\2 \1/')
> > > > > > 
> > > > > > You are considering only First and Last without spaces, which isn't
> > > > > > globally true.
> > > > > > 
> > > > > > 	$ echo "De Marchi, Lucas" | sed -e 's/\([^ ]*\), \(.*\)/\2 \1/')
> > > > > > 	De Lucas Marchi
> > > > > > 
> > > > > > not what we want. This yields better results since we don't have to
> > > > > > deal with the ambiguous space like in the other way around:
> > > > > > 
> > > > > > 	author_from_outlook=$(git show -s $sha1 --format="format:%an" | sed -e 's/\(.*\), \(.*\)/\2 \1/')
> > > > > 
> > > > > Ok... if I understood correctly so this seems what we want on that author_outlook so we can use
> > > > > that to check the presense on signed-off-by.. right?
> > > > 
> > > > In the case the email was unwillingly converted to "Last, First" by
> > > > outlook servers, we will have author in the "Last, First" form.
> > > > I don't think there's a safe conversion from
> > > > "First Last" -> "Last, First", so what we do is try to convert the other
> > > > way around and check if the sob matches.
> > > > 
> > > > Patches may be in this form (considering you are downloading them from
> > > > pw):
> > > > 	From: De Marchi, Lucas <lucas.demarchi at intel.com>
> > > > 	...
> > > > 	Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > > > 
> > > > So:
> > > > author="De Marchi, Lucas"
> > > > author_first_last="Lucas De Marchi"
> > > > 
> > > > And we accept any of the two to sign off
> > > > 
> > > > Or if you are getting from mailing list:
> > > > 	From: Lucas De Marchi <lucas.demarchi at intel.com>
> > > > 	...
> > > > 	Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > > > 
> > > > author="Lucas De Marchi"
> > > > author_first_last="Lucas De Marchi"
> > > > 
> > > > And we only accept in the First, Last form, because AFAIK there's no
> > > > server changing the other way around.
> > > > 
> > > > And this is what that line is doing:
> > > > 
> > > > 	$ echo "Lucas De Marchi" | sed -e 's/\(.*\), \(.*\)/\2 \1/'
> > > > 	Lucas De Marchi
> > > > 
> > > > 	$ echo "De Marchi, Lucas" | sed -e 's/\(.*\), \(.*\)/\2 \1/'
> > > > 	Lucas De Marchi
> > > > 
> > > > Renaming the variable to be author_first_last I think would make it
> > > > clearer.
> > > 
> > > And now I see we have patches from José:
> > > José Roberto de Souza <jose.souza at intel.com>
> > > 
> > > And replies from him coming from outlook/ews are in the form "Souza,
> > > José".  So if you are downloading the patch from patchwork, even this
> > > workaround would not fix it.
> > 
> > So, just revert that commit that introduced the outlook_author?
> > 
> > > 
> > > Maybe the only real solution is to fix patchwork.
> > 
> > and this...
> 
> I think we can try to match for either mail address matching or name
> matching (for the people with too many email addresses). When patchwork
> mangles the name, it won't change the address itself, right?

Right. What about this:

-----8<-----
From: Lucas De Marchi <lucas.demarchi at intel.com>
Subject: Accept sign-off by email or name

Stop trying to accept the mangled name from outlook/patchwork and
instead accept the sign-off if email matches.

Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>

diff --git a/dim b/dim
index ed26033f5aba..0a94b4284d80 100755
--- a/dim
+++ b/dim
@@ -730,12 +730,11 @@ function checkpatch_commit_push
 
 	# use real names for people with many different email addresses
 	author=$(git show -s $sha1 --format="format:%an")
+	author_email=$(git show -s $sha1 --format="format:%ae")
 	committer=$(git show -s $sha1 --format="format:%cn")
-	# outlook mangles mails into "Last, First"
-	author_outlook=$(git show -s $sha1 --format="format:%an" | sed -e 's/\([^ ]*\) \(.*\)/\2, \1/')
 
-	# check for author sign-off
-	if ! git show -s $sha1 | grep -qi "S.*-by:.*\\($author\\|$author_outlook\\)" ; then
+	# check for author sign-off: accept either email or name
+	if ! git show -s $sha1 | grep -qi "S.*-by:.*\\($author\\|<$author_email>\\)\$"; then
 		warn_or_fail "$sha1 is lacking author of sign-off"
 	fi
 
----->8-----


Lucas De Marchi