Merge lp://staging/~ceejatec/sfbugs2launchpad/user-mapping into lp://staging/sfbugs2launchpad

Hi Chris,

lets me apologize that it needed ten days that somebody from the LP
engineering team took a look at your exciting work.

Aside from the new features which are very useful I appreciate very
much that you also wrote a good README file and added the Relax-NG
schema files.

I have a few comments on details of your changes (nothing serious,
see below), but my main concern is that the script is becoming
complex enough so that new features should be tested. (Well... the
entire code should be tested, but that's something for other branches ;)
If you don't have enough time to write some tests (which I would
understand quite well) I would file a bug roughly like "there is
great work waiting to be merged, once we have some tests" and
hopefully someone from the LP core team can take care of it soon.

And there is one blocker: Canonical requires that external
contributors sign a "Contributor License Agreement":
http://www.canonical.com/contributors . so, could you please sign it?

> @@ -224,14 +304,29 @@
> return date.isoformat() + 'Z'
>
>
> -def create_launchpad_issue_tree(issues, options):
> +def create_launchpad_issue_tree(issues, options, users):
> root = ET.Element("launchpad-bugs")
> root.attrib['xmlns'] = "https://launchpad.net/xmlns/2006/bugs"
>
> - def _sfuser(elem, name):
> - name = (name if name is not None else "noone").strip()
> - elem.text = name
> - elem.attrib["email"] = "%<email address hidden>" % elem.text
> + def _mapsfuser(elem, sfusername):
> + """
> + Populates the given element as a Launchpad "person", with the
> + full name as the text along with "email" and (optionally)
> + "name" attributes, where "name" is the Launchpad username. If
> + the sfusername is found in the "users" map, the values will
> + come from there; otherwise we'll make something up.
> + """
> + if (sfusername in users):

[My comment is perhaps caused by the fact that I had recently used
an object from the "surrounding function" in a local function
inadvertently...] I would prefer if users is passsed as a parameter
to _mapsfuser(). But consider this just as a suggestion ;)

> + userdata = users[sfusername]
> + elem.text = userdata.full_name
> + elem.attrib["email"] = userdata.email
> + if ("lp_user_name" in userdata):
> + elem.attrib["name"] = userdata.lp_user_name

I think lp_user_name should not appear in userdata, as such an element
is not specified in the new Relax-NG file sf-user-map.rnc. Also,
the email address provides equivalent information. So I think we don't
need the two lines above.

> + else:
> + # We have no source of information about this Sourceforge username
> + name = (sfusername if sfusername is not None else "noone").strip()
> + elem.text = name
> + elem.attrib["email"] = "%<email address hidden>" % elem.text
>
> for idx,issue in enumerate(issues):
> print "Handling issue %i/%i (%s) ..." % (idx+1, len(issues), issue.id)
> @@ -248,9 +343,15 @@
> ET.SubElement(bug, "title").text = issue.summary
> ET.SubEle...

There is a new version of the bug-export.rnc file now in the Launchpad
tree.

If this is still here next week when I'm on-call reviewer I'll pick it
up and get it into shape to land, including adding some tests.

Gavin, if you're happy to get this landed I'm going to mark this as work-in-progress so that it's not sticking at the top of the review queue. I'll assign the review to you.