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	ID
Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	5c5dc96592a51b47e86c245a192aa15e3e05de85

Summary

Add additional validation for Git and SVN repos.

We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.

5c5dc96592a51b47e86c245a192aa15e3e05de85

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)
The issue has been dropped. Show all issues
```
continuation line unaligned for hanging indent

Column: 23
Error code: E131
```

Commits:

	Summary	ID
	Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	c02bbc612cfd8372fe475ff44978ac978e1e0c8b
	Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	e8e46b1bfd3ee093f855ba77e0f88d7880a04464

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)
The issue has been resolved. Show all issues
```
Missing Version Added.
```
reviewboard/scmtools/git.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This is missing typing.
```
reviewboard/scmtools/git.py (Diff revision 2)
The issue has been resolved. Show all issues
```
We should also allow file: schemes.
```
reviewboard/scmtools/git.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's link to the relevant docs here.
```
reviewboard/scmtools/svn/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing Version Added.
```
reviewboard/scmtools/svn/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This is missing typing.
```
reviewboard/scmtools/svn/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing blank line.
```

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

The issue has been resolved. 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)

The issue has been resolved. 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)
The issue has been resolved. Show all issues
```
No . in unit test docstrings.
```
reviewboard/scmtools/tests/test_svn.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Leftover debug output.
```

Commits:

	Summary	ID
	Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	e8e46b1bfd3ee093f855ba77e0f88d7880a04464
	Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	ac2dc56a105bce0353bc5a0474b9bba953c374dd

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)
The issue has been dropped. Show all issues
```
line too long (83 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/scmtools/git.py (Diff revision 3)
The issue has been dropped. Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/scmtools/tests/test_git.py (Diff revision 3)
The issue has been dropped. Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/scmtools/tests/test_git.py (Diff revision 3)
The issue has been dropped. 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)
The issue has been dropped. 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)
The issue has been dropped. Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```
reviewboard/scmtools/tests/test_git.py (Diff revision 3)
The issue has been dropped. Show all issues
```
line too long (80 > 79 characters)

Column: 80
Error code: E501
```

Commits:

	Summary	ID
	Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	ac2dc56a105bce0353bc5a0474b9bba953c374dd
	Add additional validation for Git and SVN repos. We have two cases that have come up semi-frequently, tripping up users when they're trying to configure their repositories: 1. Users were putting in a raw file path to an SVN repository, where it needs a `file://` URL. 2. 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.	5c5dc96592a51b47e86c245a192aa15e3e05de85

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)
The issue has been resolved. Show all issues
```
This should be typed as Dict[str, Any].
```
reviewboard/scmtools/svn/__init__.py (Diff revision 4)
The issue has been resolved. Show all issues
```
This should be typed as Dict[str, Any].
```

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (bcc85a0)