Code review comment for lp:~salgado/launchpad/lp-as-openid-rp

Thank you, Guilherme, this is a great step forward.

I didn't get too far on this today. Hopefully I'll have some time tomorrow: I'll resume on line 336, and actually try the server out on my system, per your instructions.

I only have one trivial comment so far. In lib/lp/testopenid/browser/configure.zcml I had a harder time reading it than I needed to because you switched back and forth from relative and absolute syntaxes. Here's an example:

279 + <browser:page
280 + for="..interfaces.server.ITestOpenIDApplication"
281 + class="lp.testopenid.browser.server.TestOpenIDView"
282 + permission="zope.Public"
283 + name="+openid"
284 + />
285 + <browser:page
286 + for="..interfaces.server.ITestOpenIDApplication"
287 + class=".server.TestOpenIDIndexView"
288 + permission="zope.Public"
289 + name="+index"
290 + />

Unless I misread it, the "server" module in lines 281 and 287 are the same, which is not as obvious as it could be. I don't care whether you choose relative or absolute, but sticking with one, particularly for the same module within the same zcml file, would help readability. It seems you mostly use relative, so I'd be inclined to standardize on that.

(I also hate browser:page directives because of the class mixin magic they do, but I won't choose you as the target for that rant. What you have done has precedence in Launchpad and is fine.)