luke@xxxxxxxxxxx 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:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html