add new command rbt list-repos to list repositories configured on a server

Review Request #6526 — Created Oct. 29, 2014 and discarded

delyn
RBTools
master
rbtools

When reviewboard server contains several repositories (currently we get 400)
it is hard to know which already exists and which are accessible for a user.
To prevent granting all users to staff, we propose this new rbtools command.
We expect people could be interested in such command and maybe someone extend
it (filtering options, improve formatting ...)

can list repositories from any working directory (inside or outside a local scm repository)

Repositories on https://reviews.reviewboard.org:
tool | name                | path                                              
-----+---------------------+---------------------------------------------------
Git  | Djblets             | git://github.com/djblets/djblets.git              
[...]
Git  | Reviewboard Persona | git://github.com/smacleod/reviewboard-persona.git
[...]
Description From Last Updated

For the rbtools master branch, any new files should have from __future__ import unicode_literals as the first line (followed by ...

daviddavid

I don't see a ton of reason to have this be a separate method instead of just putting this code ...

daviddavid

You should add from __future__ import print_statement at the top and then use print(...), for Python 3.x compatibility. It would ...

daviddavid

Can you add from six.moves import input and change raw_input to input?

daviddavid

Can you have this be the first thing that the main() method does? It's odd to prompt the user for ...

daviddavid

'json' imported but unused

reviewbotreviewbot

Col: 52 E231 missing whitespace after ','

reviewbotreviewbot

Col: 65 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 19 E211 whitespace before '('

reviewbotreviewbot

'json' imported but unused

reviewbotreviewbot

These should be in alphabetical order.

brenniebrennie

'json' imported but unused

reviewbotreviewbot

This could be: pattern = ' | '.join([ '%%-%ds' % len for len in col_len ])

daviddavid

With new rbtools code, this can be: for repo in repositories.all_items: ...

daviddavid

'json' imported but unused

reviewbotreviewbot

Col: 13 E126 continuation line over-indented for hanging indent

reviewbotreviewbot

Col: 13 E123 closing bracket does not match indentation of opening bracket's line

reviewbotreviewbot

For pep-8 formatting, the six import should come first, followed by a blank line, then the rbtools import.

daviddavid

This isn't quite as efficient as it could be (we convert types and compute length multiple times). How about: column_widths ...

daviddavid

Let's just put this inline into the print() call instead of assigning it first to a variable.

daviddavid

It would be nice to capitalize the words in the header.

daviddavid

Can we use repo.get(x, '')? It's a little bit more explicit and error-safe.

daviddavid
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
  2. setup.py (Diff revision 1)
     
     
     'json' imported but unused
    
    1. false positive, json or simplejson are used by api

  3. 
      
david
  1. 
      
  2. rbtools/commands/list_repos.py (Diff revision 1)
     
     

    For the rbtools master branch, any new files should have from __future__ import unicode_literals as the first line (followed by a blank line, then any additional imports)

  3. rbtools/commands/list_repos.py (Diff revision 1)
     
     

    I don't see a ton of reason to have this be a separate method instead of just putting this code at the end of main()

  4. rbtools/commands/list_repos.py (Diff revision 1)
     
     
     
     
     
     
     
     
     

    You should add from __future__ import print_statement at the top and then use print(...), for Python 3.x compatibility.

    It would also be very nice to have this check the length of each of the entries and print out a nicely-formatted table, rather than just using a single tab.

    1. I suppose you mean print_function ?

  5. rbtools/commands/list_repos.py (Diff revision 1)
     
     

    Can you add from six.moves import input and change raw_input to input?

  6. rbtools/commands/list_repos.py (Diff revision 1)
     
     
     

    Can you have this be the first thing that the main() method does? It's odd to prompt the user for the server URL and then error out on args.

  7. 
      
DE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
  2. rbtools/commands/list_repos.py (Diff revision 2)
     
     
    Col: 52
     E231 missing whitespace after ','
    
  3. rbtools/commands/list_repos.py (Diff revision 2)
     
     
    Col: 65
     E231 missing whitespace after ':'
    
  4. rbtools/commands/list_repos.py (Diff revision 2)
     
     
    Col: 19
     E211 whitespace before '('
    
  5. setup.py (Diff revision 2)
     
     
     'json' imported but unused
    
  6. 
      
DE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
  2. setup.py (Diff revision 3)
     
     
     'json' imported but unused
    
  3. 
      
brennie
  1. 
      
  2. rbtools/commands/list_repos.py (Diff revision 3)
     
     

    These should be in alphabetical order.

  3. 
      
DE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
  2. setup.py (Diff revision 4)
     
     
     'json' imported but unused
    
  3. 
      
david
  1. 
      
  2. rbtools/commands/list_repos.py (Diff revision 4)
     
     
     
     
     

    This could be:

    pattern = ' | '.join([
        '%%-%ds' % len
        for len in col_len
        ])
    
  3. rbtools/commands/list_repos.py (Diff revision 4)
     
     
     
     
     
     
     
     
     

    With new rbtools code, this can be:

    for repo in repositories.all_items:
        ...
    
    1. since which rbtools version, can I get access to repositoires.all_items ?

    2. ok, it is only in master branch, neither in release-0.7.x nor in release-0.6.x

  4. 
      
DE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
  2. rbtools/commands/list_repos.py (Diff revision 5)
     
     
    Col: 13
     E126 continuation line over-indented for hanging indent
    
  3. rbtools/commands/list_repos.py (Diff revision 5)
     
     
    Col: 13
     E123 closing bracket does not match indentation of opening bracket's line
    
  4. 
      
DE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        rbtools/commands/list_repos.py
        setup.py
    
    
  2. 
      
david
  1. 
      
  2. rbtools/commands/list_repos.py (Diff revision 6)
     
     
     

    For pep-8 formatting, the six import should come first, followed by a blank line, then the rbtools import.

  3. rbtools/commands/list_repos.py (Diff revision 6)
     
     
     
     
     

    This isn't quite as efficient as it could be (we convert types and compute length multiple times). How about:

    column_widths = [
       max([len(six.text_type(row[i])) for row in rows])
       for i in range(len(rows[0]))
    ]
    
  4. rbtools/commands/list_repos.py (Diff revision 6)
     
     

    Let's just put this inline into the print() call instead of assigning it first to a variable.

  5. rbtools/commands/list_repos.py (Diff revision 6)
     
     

    It would be nice to capitalize the words in the header.

  6. rbtools/commands/list_repos.py (Diff revision 6)
     
     

    Can we use repo.get(x, '')? It's a little bit more explicit and error-safe.

  7. 
      
david
Review request changed

Status: Discarded

Loading...