Code review comment for lp:~leonardr/launchpad/rename-grant-permissions

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Leonard,

This is a cool feature. Here are my comments on the +authorize-token page's text.

https://launchpad.dev/+authorize-token says:
>The Ubuntu computer identified as Edwin's Computer wants access to your
>Launchpad account. If you allow the integration, all applications
>running on Edwin's Computer will have read-write access to your
>Launchpad account, including to your private data.

I like this paragraph.

>If Edwin's Computer is not the computer you're using right now, or if
>you don't trust this computer, you should click "No, thanks, I don't
>trust this computer", or close this window now.

I think this feature will increase the likelihood that less
computer-savvy users will be using apps that need API tokens, and I
think that the concept of whether they "trust this computer" could be
confusing. How about this alternative:
"... or if untrusted users can access Edwin's Computer, ..."

>Even if you decide to allow the integration, you can change your mind
>later.
>
> [Give all programs running on "Edwin's Computer" access to my Launchpad account.]

This sentence is hard to parse and it is difficult to rewrite since "give to" and
"access to" make the "to" preposition ambiguous. How about:
[Allow all programs running on "Edwin's Computer" to access my Launchpad account.]

> [No, thanks, I don't trust this computer.]

Just in case the user is confused about wheter "Edwin's Computer" is the same as
"this computer", it might be better to say:
[No, thanks, I don't trust "Edwin's Computer"]

Even though I complained above about the concept of trusting a computer,
I think it is reasonable here since the above comments explain the criteria
for determing that. Otherwise, the button will get even more verbose.

Since I'm not a graduated UI reviewer, you will still need one more UI
review, and you might want to see if they disagree with any of my
suggestions before you make any changes.

-Edwin

review: Approve (ui*)

« Back to merge proposal