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

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

james909fdsa
Review Board
release-2.5.x
d1d7eb0...
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
Review request changed

Testing Done:

~  

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

  ~

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

-   It ran 2569 tests in 293.380 seconds.

   
   

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.

Commit:

-ae97f0702c9c41538ff1159f60772e617b699c1f
+d1d7eb054a5e3c5aaedd2f4e6e8908be0e4d217a

Diff:

Revision 3 (+119 -24)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...