Fish Trophy

benbenntt got a fish trophy!

Fish Trophy

git http or https remote with username name in remote url

Review Request #1771 — Created Sept. 8, 2010 and discarded

Information

RBTools

Reviewers

issue 1803  git http /ftps / https remote origin with username name in remote url. http://code.google.com/p/reviewboard/issues/detail?id=1803

Added reviewboard.repo.url  configuration option to set the repo url to use when posting
a review.
Usage:
git config reviewboard.repo.url 'https://whatismyrepourl/path/.git'

Added --remove-username
If url contains a username, it removes the username when
posting a review.
Example:
https://myusername@whatismyrepourl/path/.git
becomes
https://whatismyrepourl/path/.git

 
BE
  1. 
      
  2. rbtools/postreview.py (Diff revision 2)
     
     
    probably should check for ftp also ?
  3. 
      
david
  1. 
      
  2. rbtools/postreview.py (Diff revision 2)
     
     
    You don't need the parentheses here. I'd also like it if it was explicit about the different prefixes:
    
    if origin_url.startswith('http://') or origin_url.startswith('https://'):
    
    You don't need ftp, because as far as I know, there's no git ftp transport :)
  3. rbtools/postreview.py (Diff revision 2)
     
     
    No parentheses around the if conditions here.
  4. 
      
BE
BE
BE
BE
  1. 
      
  2. rbtools/postreview.py (Diff revision 5)
     
     
    wrong return comment.
  3. 
      
BE
OR
  1. Works for me against a 1.5 RC1 server.
  2. rbtools/postreview.py (Diff revision 6)
     
     
     
    There needs to be a space added at the end of the first line.
  3. 
      
chipx86
  1. 
      
  2. rbtools/postreview.py (Diff revision 6)
     
     
    Single _, and no trailing __.
    
    Space before "url"
  3. rbtools/postreview.py (Diff revision 6)
     
     
     
     
     
     
     
     
    The first line should always be a one-line summary.
    
    Then a blank line, followed by any human-readable documentation.
    
    The information about Python versions would belong as comments, not in the documentation. However, we absolutely must support Python 2.4, so you should use the old version of the results from urlparse.
  4. rbtools/postreview.py (Diff revision 6)
     
     
    A more appropriate name may be "url_schemas".
  5. rbtools/postreview.py (Diff revision 6)
     
     
     
    Blank line before the commets.
    
    Comments should always be in sentence casing, and must have a space between the "#" and text.
  6. rbtools/postreview.py (Diff revision 6)
     
     
     
    Blank line between these.
  7. rbtools/postreview.py (Diff revision 6)
     
     
    This is sort of weird. Please do:
    
    for url_schema in url_schemas:
        if url.startswith(url_schema):
            ...
  8. rbtools/postreview.py (Diff revision 6)
     
     
     
    Can be combined to one line.
    
    Parameters must always have a space between the preceding comma and the parameter. So, url.replace(url, http)
  9. rbtools/postreview.py (Diff revision 6)
     
     
    Space around operators.
    
    pr.username + '@', '', 1
  10. rbtools/postreview.py (Diff revision 6)
     
     
     
    Blank line between these.
  11. rbtools/postreview.py (Diff revision 6)
     
     
    Space around =
  12. rbtools/postreview.py (Diff revision 6)
     
     
     
    Blank line between these.
  13. rbtools/postreview.py (Diff revision 6)
     
     
    Space around =
  14. rbtools/postreview.py (Diff revision 6)
     
     
     
     
     
     
     
    So, I'd actually rather not go this route of having the option, even if it's configurable. It means people have to do a lot more manual work to get going.
    
    Instead, it'd be much better for Review Board to try two versions of the URL. One with the username, one without. There are other SCMClients (such as the Perforce one) that do a variety of checks for repository names.
  15. rbtools/tests.py (Diff revision 6)
     
     
    Only special operator functions built-in to Python can have the __foo__ naming convention. This should be _get_urls.
  16. rbtools/tests.py (Diff revision 6)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    No need for the \
  17. rbtools/tests.py (Diff revision 6)
     
     
     
    The previous statement should have just returned this instead of storing in a variable first.
  18. rbtools/tests.py (Diff revision 6)
     
     
     
     
     
     
    Unit test docstrings must always be only one line that briefly describes the unit test. Same line as the """. This is what we'll show when verbosely showing the unit tests that are running.
  19. rbtools/tests.py (Diff revision 6)
     
     
    Same comments about the comment.
  20. rbtools/tests.py (Diff revision 6)
     
     
     
    Blank line between these.
  21. 
      
BE
Review request changed
Status:
Discarded