Summary

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

Review Request #6526 — Created Oct. 29, 2014 and discarded Jan. 29, 2019, 1:41 p.m.

Information

Owner

delyn

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

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 ...)

Testing Done

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
[...]

Issues

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 …	david	Jan. 3, 2015, 7:32 a.m.
I don't see a ton of reason to have this be a separate method instead of just putting this code …	david	Jan. 3, 2015, 7:32 a.m.
You should add from __future__ import print_statement at the top and then use print(...), for Python 3.x compatibility. It would …	david	Jan. 3, 2015, 7:33 a.m.
Can you add from six.moves import input and change raw_input to input?	david	Jan. 3, 2015, 7:33 a.m.
Can you have this be the first thing that the main() method does? It's odd to prompt the user for …	david	Jan. 3, 2015, 7:33 a.m.
'json' imported but unused	reviewbot	Nov. 4, 2014, 7:45 a.m.
Col: 52 E231 missing whitespace after ','	reviewbot	Jan. 3, 2015, 7:38 a.m.
Col: 65 E231 missing whitespace after ':'	reviewbot	Jan. 3, 2015, 7:38 a.m.
Col: 19 E211 whitespace before '('	reviewbot	Jan. 3, 2015, 7:38 a.m.
'json' imported but unused	reviewbot	Jan. 3, 2015, 7:38 a.m.
These should be in alphabetical order.	brennie	Jan. 14, 2015, 7:52 a.m.
'json' imported but unused	reviewbot	Jan. 3, 2015, 7:38 a.m.
This could be: pattern = ' \| '.join([ '%%-%ds' % len for len in col_len ])	david	Jan. 18, 2016, 6:01 a.m.
With new rbtools code, this can be: for repo in repositories.all_items: ...	david	Jan. 18, 2016, 6:25 a.m.
'json' imported but unused	reviewbot	Jan. 14, 2015, 7:52 a.m.
Col: 13 E126 continuation line over-indented for hanging indent	reviewbot	Jan. 18, 2016, 6:41 a.m.
Col: 13 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Jan. 18, 2016, 6:41 a.m.
For pep-8 formatting, the six import should come first, followed by a blank line, then the rbtools import.	david	Jan. 19, 2016, 4:34 p.m.
This isn't quite as efficient as it could be (we convert types and compute length multiple times). How about: column_widths …	david	Jan. 19, 2016, 4:34 p.m.
Let's just put this inline into the print() call instead of assigning it first to a variable.	david	Jan. 19, 2016, 4:34 p.m.
It would be nice to capitalize the words in the header.	david	Jan. 19, 2016, 4:34 p.m.
Can we use repo.get(x, '')? It's a little bit more explicit and error-safe.	david	Jan. 19, 2016, 4:34 p.m.

Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/list_repos.py
    setup.py



Tool: Pyflakes
Processed Files:
    rbtools/commands/list_repos.py
    setup.py

setup.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'json' imported but unused
```
1. DE
  
  delyn Nov. 4, 2014, 7:45 a.m.
  false positive, json or simplejson are used by api

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

The issue has been resolved. Show all issues

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)

The issue has been resolved. Show all issues

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)

The issue has been resolved. Show all issues

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.

DE

delyn Jan. 3, 2015, 7:33 a.m.
```
I suppose you mean print_function ?
```

rbtools/commands/list_repos.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you add from six.moves import input and change raw_input to input?
```

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

The issue has been resolved. Show all issues

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:

-	86c43ea69b18fce70d315ab7a16e35cd6440d0b8
+	68c2156b16230d7481dafe1c77a2b29317570bc2

Diff:

Revision 2 (+68)

Show changes

	setup.py
	rbtools/commands/list_repos.py

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 2)
The issue has been resolved. Show all issues
```
Col: 52
 E231 missing whitespace after ','
```
rbtools/commands/list_repos.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 65
 E231 missing whitespace after ':'
```
rbtools/commands/list_repos.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Col: 19
 E211 whitespace before '('
```
setup.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'json' imported but unused
```

Change Summary:

fix reviewbot issues

Testing Done:

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

~		Repositories:
~		Name Path SCM
~		gitolite:repo1 git@gitolite.company.com:repo1 Git
~		/vobs/myvob /view/reviewboard_view/vobs/myvob ClearCase
	~	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
		[...]

Diff:

Revision 3 (+69)

Show changes

	setup.py
	rbtools/commands/list_repos.py

Tool: Pyflakes
Processed Files:
    rbtools/commands/list_repos.py
    setup.py

setup.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'json' imported but unused
```

rbtools/commands/list_repos.py (Diff revision 3)
The issue has been resolved. Show all issues
```
These should be in alphabetical order.
```

Change Summary:

fix import order

Commit:

-	68c2156b16230d7481dafe1c77a2b29317570bc2
+	6ebf6f53c3a221ca2693c959476cf429e3079efb

Diff:

Revision 4 (+69)

Show changes

	setup.py
	rbtools/commands/list_repos.py

Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/list_repos.py
    setup.py



Tool: Pyflakes
Processed Files:
    rbtools/commands/list_repos.py
    setup.py

setup.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'json' imported but unused
```

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

The issue has been resolved. Show all issues

This could be:

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

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

The issue has been resolved. Show all issues

With new rbtools code, this can be:

for repo in repositories.all_items:
    ...

delyn Jan. 18, 2016, 6:03 a.m.

since which rbtools version, can I get access to repositoires.all_items ?

delyn Jan. 18, 2016, 6:25 a.m.

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

Change Summary:

update according to David remarks on diff-r4

Branch:

-	release-0.6.x
+	master

Diff:

Revision 5 (+66)

Show changes

	setup.py
	rbtools/commands/list_repos.py

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)
The issue has been resolved. Show all issues
```
Col: 13
 E126 continuation line over-indented for hanging indent
```

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

The issue has been resolved. Show all issues

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

Change Summary:

fix reviewbot findings

Diff:

Revision 6 (+63)

Show changes

	setup.py
	rbtools/commands/list_repos.py

Tool: Pyflakes
Processed Files:
    rbtools/commands/list_repos.py
    setup.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/commands/list_repos.py
    setup.py

Fix it!

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

An issue was opened. Show all issues

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)

An issue was opened. Show all issues

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)

An issue was opened. Show all issues

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)
An issue was opened. Show all issues
```
It would be nice to capitalize the words in the header.
```

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

An issue was opened. Show all issues

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

Review Board 7.0 alpha 0 (dev)

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

Information

Reviewers

Change Summary:

Commit:

Diff:

Change Summary:

Testing Done:

Diff:

Change Summary:

Commit:

Diff:

Change Summary:

Branch:

Diff:

Change Summary:

Diff:

Status: Discarded