Summary:

Add additional validation for Git and SVN repos.

Review Request #13223 — Created Aug. 18, 2023 and submitted Aug. 21, 2023, 3:55 p.m.

Information
Owner:	david
Repository:	Review Board
Branch:	release-6.x
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

We have two cases that have come up semi-frequently, tripping up users
when they're trying to configure their repositories:

Users were putting in a raw file path to an SVN repository, where it
needs a file:// URL.
When configuring a Git repo, they'd enter their SVN clone address but
leave out the raw file URL. This one is especially confusing because
a lot of people expect Git to just work, and don't read our
documentation about why it doesn't.

This change adds new repository form subclasses that can validate these
conditions.

Testing Done


Verified that I wasn't able to save an SVN repo pointing to a bare

  filesystem path, but could with file:// and https://
Verified that trying to save a Git repo with a path but no raw file

  URL would fail.
Made sure that saving with a raw file URL or Git repos on a hosting

  service still worked correctly.
Ran unit tests.

Commits

Summary
	Add additional validation for Git and SVN repos.

Screenshots

Issues

Description	From	Last Updated
continuation line unaligned for hanging indent Column: 23 Error code: E131	reviewbot	Aug. 18, 2023, 2:09 p.m.
Missing Version Added.	chipx86	Aug. 20, 2023, 12:24 p.m.
This is missing typing.	chipx86	Aug. 20, 2023, 12:24 p.m.
We should also allow file: schemes.	chipx86	Aug. 20, 2023, 12:24 p.m.
Let's link to the relevant docs here.	chipx86	Aug. 20, 2023, 12:30 p.m.
Missing Version Added.	chipx86	Aug. 20, 2023, 12:30 p.m.
This is missing typing.	chipx86	Aug. 20, 2023, 12:30 p.m.
Missing blank line.	chipx86	Aug. 20, 2023, 12:30 p.m.
Should add a -> None so we can opt into typing. Also, let's add tests for supported schemes, just to …	chipx86	Aug. 21, 2023, 8:41 a.m.
Should add a -> None so we can opt into typing. Also, let's add tests for other schemes, just to …	chipx86	Aug. 21, 2023, 9:14 a.m.
No . in unit test docstrings.	chipx86	Aug. 20, 2023, 12:32 p.m.
Leftover debug output.	chipx86	Aug. 20, 2023, 12:32 p.m.
line too long (83 > 79 characters) Column: 80 Error code: E501	reviewbot	Aug. 21, 2023, 9:16 a.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Aug. 21, 2023, 9:16 a.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Aug. 21, 2023, 9:16 a.m.
local variable 'x' is assigned to but never used Column: 9 Error code: F841	reviewbot	Aug. 21, 2023, 9:16 a.m.
local variable 'y' is assigned to but never used Column: 9 Error code: F841	reviewbot	Aug. 21, 2023, 9:16 a.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Aug. 21, 2023, 9:16 a.m.
line too long (80 > 79 characters) Column: 80 Error code: E501	reviewbot	Aug. 21, 2023, 9:16 a.m.
This should be typed as Dict[str, Any].	chipx86	Aug. 21, 2023, 3:48 p.m.
This should be typed as Dict[str, Any].	chipx86	Aug. 21, 2023, 3:48 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/scmtools/svn/__init__.py (Diff revision 1)

Show all issues

continuation line unaligned for hanging indent

Column: 23
Error code: E131

Commits:

	Summary
-		Add additional validation for Git and SVN repos.
+		Add additional validation for Git and SVN repos.

Diff:

Revision 2 (+210 -16)

Show changes

	reviewboard/scmtools/git.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/tests/test_git.py
	reviewboard/scmtools/tests/test_repository_form.py
	reviewboard/scmtools/tests/test_svn.py
	reviewboard/webapi/tests/test_repository.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/scmtools/git.py (Diff revision 2)
Show all issues
```
Missing Version Added.
```
reviewboard/scmtools/git.py (Diff revision 2)
Show all issues
```
This is missing typing.
```
reviewboard/scmtools/git.py (Diff revision 2)
Show all issues
```
We should also allow file: schemes.
```
reviewboard/scmtools/git.py (Diff revision 2)
Show all issues
```
Let's link to the relevant docs here.
```
reviewboard/scmtools/svn/__init__.py (Diff revision 2)
Show all issues
```
Missing Version Added.
```
reviewboard/scmtools/svn/__init__.py (Diff revision 2)
Show all issues
```
This is missing typing.
```
reviewboard/scmtools/svn/__init__.py (Diff revision 2)
Show all issues
```
Missing blank line.
```

reviewboard/scmtools/tests/test_git.py (Diff revision 2)

Show all issues

Should add a -> None so we can opt into typing.

Also, let's add tests for supported schemes, just to make sure we don't ever regress.

reviewboard/scmtools/tests/test_svn.py (Diff revision 2)

Show all issues

Should add a -> None so we can opt into typing.

Also, let's add tests for other schemes, just to make sure we don't ever regress that.

reviewboard/scmtools/tests/test_svn.py (Diff revision 2)
Show all issues
```
No . in unit test docstrings.
```
reviewboard/scmtools/tests/test_svn.py (Diff revision 2)
Show all issues
```
Leftover debug output.
```

Commits:

	Summary
-		Add additional validation for Git and SVN repos.
+		Add additional validation for Git and SVN repos.

Diff:

Revision 3 (+392 -24)

Show changes

	reviewboard/scmtools/git.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/tests/test_git.py
	reviewboard/scmtools/tests/test_repository_form.py
	reviewboard/scmtools/tests/test_svn.py
	reviewboard/webapi/tests/test_repository.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/scmtools/git.py (Diff revision 3)

Show all issues

line too long (83 > 79 characters)

Column: 80
Error code: E501

reviewboard/scmtools/git.py (Diff revision 3)

Show all issues

line too long (80 > 79 characters)

Column: 80
Error code: E501

reviewboard/scmtools/tests/test_git.py (Diff revision 3)

Show all issues

line too long (80 > 79 characters)

Column: 80
Error code: E501

reviewboard/scmtools/tests/test_git.py (Diff revision 3)

Show all issues

local variable 'x' is assigned to but never used

Column: 9
Error code: F841

reviewboard/scmtools/tests/test_git.py (Diff revision 3)

Show all issues

local variable 'y' is assigned to but never used

Column: 9
Error code: F841

reviewboard/scmtools/tests/test_git.py (Diff revision 3)

Show all issues

line too long (80 > 79 characters)

Column: 80
Error code: E501

reviewboard/scmtools/tests/test_git.py (Diff revision 3)

Show all issues

line too long (80 > 79 characters)

Column: 80
Error code: E501

Commits:

	Summary
-		Add additional validation for Git and SVN repos.
+		Add additional validation for Git and SVN repos.

Diff:

Revision 4 (+388 -28)

Show changes

	reviewboard/scmtools/git.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/tests/test_git.py
	reviewboard/scmtools/tests/test_repository_form.py
	reviewboard/scmtools/tests/test_svn.py
	reviewboard/webapi/tests/test_repository.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/scmtools/git.py (Diff revision 4)
Show all issues
```
This should be typed as Dict[str, Any].
```
reviewboard/scmtools/svn/__init__.py (Diff revision 4)
Show all issues
```
This should be typed as Dict[str, Any].
```

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (bcc85a0)