add new command rbt list-repos to list repositories configured on a server
Review Request #6526 — Created Oct. 29, 2014 and discarded
Information | |
---|---|
delyn | |
RBTools | |
master | |
Reviewers | |
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 … |
|
|
I don't see a ton of reason to have this be a separate method instead of just putting this code … |
|
|
You should add from __future__ import print_statement at the top and then use print(...), for Python 3.x compatibility. It would … |
|
|
Can you add from six.moves import input and change raw_input to input? |
|
|
Can you have this be the first thing that the main() method does? It's odd to prompt the user for … |
|
|
'json' imported but unused |
![]() |
|
Col: 52 E231 missing whitespace after ',' |
![]() |
|
Col: 65 E231 missing whitespace after ':' |
![]() |
|
Col: 19 E211 whitespace before '(' |
![]() |
|
'json' imported but unused |
![]() |
|
These should be in alphabetical order. |
|
|
'json' imported but unused |
![]() |
|
This could be: pattern = ' | '.join([ '%%-%ds' % len for len in col_len ]) |
|
|
With new rbtools code, this can be: for repo in repositories.all_items: ... |
|
|
'json' imported but unused |
![]() |
|
Col: 13 E126 continuation line over-indented for hanging indent |
![]() |
|
Col: 13 E123 closing bracket does not match indentation of opening bracket's line |
![]() |
|
For pep-8 formatting, the six import should come first, followed by a blank line, then the rbtools import. |
|
|
This isn't quite as efficient as it could be (we convert types and compute length multiple times). How about: column_widths … |
|
|
Let's just put this inline into the print() call instead of assigning it first to a variable. |
|
|
It would be nice to capitalize the words in the header. |
|
|
Can we use repo.get(x, '')? It's a little bit more explicit and error-safe. |
|
-
-
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) -
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()
-
rbtools/commands/list_repos.py (Diff revision 1) You should add
from __future__ import print_statement
at the top and then useprint(...)
, 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.
-
rbtools/commands/list_repos.py (Diff revision 1) Can you add
from six.moves import input
and changeraw_input
toinput
? -
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.
Change Summary:
Add support for python3 using six and __future__ import as recommended in code review Add pretty_print_table function to improve output formatting
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+68) |

-
Tool: Pyflakes Processed Files: rbtools/commands/list_repos.py setup.py Tool: PEP8 Style Checker Processed Files: rbtools/commands/list_repos.py setup.py
-
-
-
-
Change Summary:
fix reviewbot issues
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+69) |
Change Summary:
fix import order
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+69) |

-
Tool: PEP8 Style Checker Processed Files: rbtools/commands/list_repos.py setup.py Tool: Pyflakes Processed Files: rbtools/commands/list_repos.py setup.py
-
-
-
rbtools/commands/list_repos.py (Diff revision 4) This could be:
pattern = ' | '.join([ '%%-%ds' % len for len in col_len ])
-
rbtools/commands/list_repos.py (Diff revision 4) With new rbtools code, this can be:
for repo in repositories.all_items: ...
Change Summary:
update according to David remarks on diff-r4
Branch: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+66) |

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

-
Tool: Pyflakes Processed Files: rbtools/commands/list_repos.py setup.py Tool: PEP8 Style Checker Processed Files: rbtools/commands/list_repos.py setup.py
-
-
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.
-
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])) ]
-
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. -
rbtools/commands/list_repos.py (Diff revision 6) It would be nice to capitalize the words in the header.
-
rbtools/commands/list_repos.py (Diff revision 6) Can we use
repo.get(x, '')
? It's a little bit more explicit and error-safe.