Add a GitHub plan to use another user's repository.

Review Request #9028 — Created June 20, 2017 and discarded

james909fdsa
Review Board
release-2.5.x
reviewboard
Previously, a user could only configure a repository within the user
or an organization. With the addition of this plan, a user will be
able to configure another user's repository by entering the user's
name and desired repository.

The error messages for all plans were also updated by using
dictionaries and %s to get the user's name and repo's name, rather
than outputting a static string.

Ran unit tests with and without parameters, and had no errors.

Manually tested user:chipx86 and repository name:baseconfigs.git,
and it worked with no errors.

When the plan is tested with random inputs for both username and
repo name, we are given the correct error message.

Description From Last Updated

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

Let's use "Owner's username"

chipx86chipx86

"org_name" should be "other_username"

chipx86chipx86

Should use "other_username" ("username" is one word). Note that this won't apply to the github_other_user_name variables below, because that's actually …

chipx86chipx86

"other_username"

chipx86chipx86

"Desired" might be confusing to a lot of users, particularly non-English speakers. "Other User's Repository" would be better.

chipx86chipx86

"username" isn't right here. Let's use "owner".

chipx86chipx86

These can be combined to one line, I believe.

chipx86chipx86

No blank line here.

chipx86chipx86

Let's extend this to say: " ... was not found, or is not publicly-accessible."

chipx86chipx86

No blank line here.

chipx86chipx86

No blank line here.

chipx86chipx86

These are only relevant to organizations. You don't want them for this plan.

chipx86chipx86

Only one blank line here.

chipx86chipx86

Can you put each argument on its own line?

chipx86chipx86

Let's swap the order of these, so that it'll match the order in which they're presented in the form. This …

chipx86chipx86
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

JA
chipx86
  1. 
      
  2. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     

    Let's use "Owner's username"

  3. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     

    "org_name" should be "other_username"

  4. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     

    Should use "other_username" ("username" is one word). Note that this won't apply to the github_other_user_name variables below, because that's actually built as github_<planid>_<fieldname>.

  5. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     

    "other_username"

  6. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     

    "Desired" might be confusing to a lot of users, particularly non-English speakers.

    "Other User's Repository" would be better.

  7. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     

    "username" isn't right here. Let's use "owner".

  8. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     

    These can be combined to one line, I believe.

  9. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     
     

    No blank line here.

  10. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     

    Let's extend this to say:

    " ... was not found, or is not publicly-accessible."

  11. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     
     

    No blank line here.

  12. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     
     

    No blank line here.

  13. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     

    These are only relevant to organizations. You don't want them for this plan.

  14. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     
     
     

    Only one blank line here.

  15. Can you put each argument on its own line?

  16. Let's swap the order of these, so that it'll match the order in which they're presented in the form. This is just for readability purposes and consistency with the other tests.

  17. 
      
JA
david
Review request changed

Status: Discarded

Loading...