Merge lp:~dktrkranz/launchpadlib/bug-643699 into lp:launchpadlib
Proposed by
Luca Falavigna
Status: | Merged |
---|---|
Merged at revision: | 138 |
Proposed branch: | lp:~dktrkranz/launchpadlib/bug-643699 |
Merge into: | lp:launchpadlib |
Diff against target: |
42 lines (+14/-2) 1 file modified
src/launchpadlib/credentials.py (+14/-2) |
To merge this branch: | bzr merge lp:~dktrkranz/launchpadlib/bug-643699 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Martin Pool (community) | Needs Fixing | ||
Review via email:
|
Description of the change
Fix for LP: #643699
To post a comment you must log in.
Thanks for submitting this branch Luca. I have a couple of comments:
We really don't need a second string (TIMEOUT_MESSAGE). The two
strings should be combined into one. The string should end in something
like this:
"Press any key to view the authentication page in a web browser."
There's no reason to have a timeout. If the intent is to avoid
censusing the user by suddenly showing them a web page, then we have no
guarantee that they'll read the message before the timeout ends. They
might be otherwise distracted when the message is shown, the message may
be generated at the end of a long shell pipeline or script that the user
has switched away from, the user may be a slow reader, etc.
Also, the code as-written won't actually proceed if they press any key,
only enter. Either the code should be changed or the prompt should be
re-worded to ask the user to press enter.
Thanks again for the branch. This bug certainly needs attention.