Code review comment for lp:~bac/launchpadlib/keyring

Nice, Brad. Thank you.

We discussed this on a phone call and IRC. The discussion had two points.

1) Why return None if we can't decode, forcing the user to get a new authorization? Why not try the undecoded string, in case this is some kind of legacy issue? Answer: deleting or changing broken passwords is annoying. It is better to simply make the user follow a known workflow to get a new authorization than to make them try to figure out how to delete broken passwords in the key. There's a nice comment for that now.

2) We talked about the marker, and whether we needed any further checks. I was convinced that this is fine. In particular, the "consumer_key" string was at the beginning, so checking for <B64> at the beginning effectively subsumes that check. We also talked about whether it was important that the marker string not be base 64 itself, and agreed that it was not important.

A small additional point: if you are going to mess with whitespace, in line 80 of the diff, don't we want a space after the colon?

  headers = {'Referer': web_root}

Do what you will. :-)

Gary