Code review comment for lp://staging/~gary/launchpad/bug553368

Revision history for this message
Brad Crittenden (bac) wrote :

Hi Gary,

Thanks for working through the considerations for this branch with Robert. I agree with the outcome and reasoning: OOPS now for our (Canonical's) SSO service problems and address the issue of other SSOs when the time comes.

<rant>
I know you're just following existing convention but I'd much rather see:

response_code = httplib.SERVICE_UNAVAILABLE

than

response_code = 503 # Service Unavailable.

I wonder why we never adopted the use of the symbols? Because the codes are ubiquitous and everyone knows them? That can't be it because I found errors in reviews of Leonard's code where he wrote the wrong number and if he couldn't get them right what chance do the rest of us have?

</rant>

How about expanding:

You can also join <a href="irc://irc.freenode.net/launchpad">the #launchpad IRC support channel on irc.freenode.net</a> for further assistance.

to

You can also join <a href="irc://irc.freenode.net/launchpad">the #launchpad IRC support channel on irc.freenode.net</a> to ask for further assistance.

It is minor but indicates they have to go there and ask.

Can "This will not work with the AppServerLayer." be checked and enforced nicely?

review: Approve (code)

« Back to merge proposal