Re: [PATCH 3/4] git-p4: importing labels should cope with missing owner

From: Pete Wyckoff <pw@padd.com>
Date: Nov 7, 2011, 22:34 UTC
Message-ID: <20111107223423.GA19362@arf.padd.com>
In-Reply-To: <1320701799-26071-4-git-send-email-luke@diamand.org>

luke@diamand.org wrote on Mon, 07 Nov 2011 21:36 +0000:

> In p4, the Owner field is optional. If it is missing,
> construct something sensible rather than crashing.

Nice.  One question:

Show 9 quoted lines

>                      owner = labelDetails["Owner"]
>                      tagger = ""
> -                    if author in self.users:
> +                    if not owner:
> +                        tagger = "%s <a@b> %s %s" % (self.p4UserId(), epoch, self.tz)
> +                    elif author in self.users:
>                          tagger = "%s %s %s" % (self.users[owner], epoch, self.tz)
>                      else:
>                          tagger = "%s <a@b> %s %s" % (owner, epoch, self.tz)

We trust p4 always returns a key "Owner", but it could be an empty string? I'm okay with that, if that's how it works.

Should be "author in self.users"? I'm looking at the creation of the committer string above, and guessing that we try to use owner, and if that doesn't exist, try to use author just like as above, so something like:

    if owner:
	if owner in self.users:
	    email = self.users[owner]
	else:
	    email = "%s <a@b>" % owner
    else:
	if author in self.users:
	    email = self.users[author]
	else:
	    email = "%s <a@b>" % author

I could misunderstand this.

Maybe that lookup in self.users[] and <a@b> default wants to have a function since it will be repeated three times now.

Ack to the whole series.

		-- Pete