Summary

Add type safety for post-commit SCM objects and fix some type issues.

Review Request #10497 — Created April 2, 2019 and submitted May 17, 2019, 2:12 a.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This adds type safety to important attributes on Branch, ChangeSet,
Commit, and Revision, forcing hosting services and SCMTools to
encode or decode data properly and pass the right types during
construction. This allows consumers of these objects to be certain about
what type they're working with, and just keeps everything nice and
consistent across implementations and Python versions.

The new type safety caught some issues in a few of our backends, where
we were passing Unicode diffs or byte string metadata. These haven't
been a problem before (and we had some backwards-compatibility support
in Commit that dealt with Unicode diffs, which are now gone). These
cases have all been updated to pass the correct types.

Testing Done

Unit tests pass on Python 2.7 and 3.7 (with other in-progress changes).

Tested browsing for commits and posting commits for review using GitHub,
Subversion, and Perforce (the three most heavily changed), complete with
diffs using Emoji characters.

Commits

Summary	ID
Add type safety for post-commit SCM objects and fix some type issues. This adds type safety to important attributes on `Branch`, `ChangeSet`, `Commit`, and `Revision`, forcing hosting services and SCMTools to encode or decode data properly and pass the right types during construction. This allows consumers of these objects to be certain about what type they're working with, and just keeps everything nice and consistent across implementations and Python versions. The new type safety caught some issues in a few of our backends, where we were passing Unicode diffs or byte string metadata. These haven't been a problem before (and we had some backwards-compatibility support in `Commit` that dealt with Unicode diffs, which are now gone). These cases have all been updated to pass the correct types.	0bbc031e7434fe7949be8659ad379caa45fe7280

Summary

Add type safety for post-commit SCM objects and fix some type issues.

This adds type safety to important attributes on `Branch`, `ChangeSet`, `Commit`, and `Revision`, forcing hosting services and SCMTools to encode or decode data properly and pass the right types during construction. This allows consumers of these objects to be certain about what type they're working with, and just keeps everything nice and consistent across implementations and Python versions. The new type safety caught some issues in a few of our backends, where we were passing Unicode diffs or byte string metadata. These haven't been a problem before (and we had some backwards-compatibility support in `Commit` that dealt with Unicode diffs, which are now gone). These cases have all been updated to pass the correct types.

0bbc031e7434fe7949be8659ad379caa45fe7280

Issues

Description	From	Last Updated
F401 'django.utils.encoding.force_str' imported but unused	reviewbot	April 2, 2019, 2:40 p.m.
Huh?	david	April 10, 2019, 4:55 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/scmtools/core.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F401 'django.utils.encoding.force_str' imported but unused
```

Change Summary:

Removed an unused import.

Commits:

	Summary	ID
	Add type safety for post-commit SCM objects and fix some type issues. This adds type safety to important attributes on `Branch`, `ChangeSet`, `Commit`, and `Revision`, forcing hosting services and SCMTools to encode or decode data properly and pass the right types during construction. This allows consumers of these objects to be certain about what type they're working with, and just keeps everything nice and consistent across implementations and Python versions. The new type safety caught some issues in a few of our backends, where we were passing Unicode diffs or byte string metadata. These haven't been a problem before (and we had some backwards-compatibility support in `Commit` that dealt with Unicode diffs, which are now gone). These cases have all been updated to pass the correct types.	df8f2fc4e5b3c0bedb498ec456485c158468f928
	Add type safety for post-commit SCM objects and fix some type issues. This adds type safety to important attributes on `Branch`, `ChangeSet`, `Commit`, and `Revision`, forcing hosting services and SCMTools to encode or decode data properly and pass the right types during construction. This allows consumers of these objects to be certain about what type they're working with, and just keeps everything nice and consistent across implementations and Python versions. The new type safety caught some issues in a few of our backends, where we were passing Unicode diffs or byte string metadata. These haven't been a problem before (and we had some backwards-compatibility support in `Commit` that dealt with Unicode diffs, which are now gone). These cases have all been updated to pass the correct types.	e94bd31d4ac6243f9f2c08fe2abbb1286cb6c898

Diff:

Revision 2 (+272 -360)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/rbgateway.py
	reviewboard/scmtools/core.py
	reviewboard/scmtools/hg.py
	reviewboard/scmtools/perforce.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/tests/test_core.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/commit_utils.py (Diff revision 2)

The issue has been resolved. Show all issues

Huh?

chipx86 April 10, 2019, 4:20 p.m.

json.dumps returns Unicode text.
b64encode requires bytes.
We need to return Unicode text.
Ugly for sure. I could separate across two lines if you'd prefer.

david April 10, 2019, 4:31 p.m.

That would make it much more readable I think.

Change Summary:

Improved readability of the validation info serialization code.

Commits:

	Summary	ID
	Add type safety for post-commit SCM objects and fix some type issues. This adds type safety to important attributes on `Branch`, `ChangeSet`, `Commit`, and `Revision`, forcing hosting services and SCMTools to encode or decode data properly and pass the right types during construction. This allows consumers of these objects to be certain about what type they're working with, and just keeps everything nice and consistent across implementations and Python versions. The new type safety caught some issues in a few of our backends, where we were passing Unicode diffs or byte string metadata. These haven't been a problem before (and we had some backwards-compatibility support in `Commit` that dealt with Unicode diffs, which are now gone). These cases have all been updated to pass the correct types.	e94bd31d4ac6243f9f2c08fe2abbb1286cb6c898
	Add type safety for post-commit SCM objects and fix some type issues. This adds type safety to important attributes on `Branch`, `ChangeSet`, `Commit`, and `Revision`, forcing hosting services and SCMTools to encode or decode data properly and pass the right types during construction. This allows consumers of these objects to be certain about what type they're working with, and just keeps everything nice and consistent across implementations and Python versions. The new type safety caught some issues in a few of our backends, where we were passing Unicode diffs or byte string metadata. These haven't been a problem before (and we had some backwards-compatibility support in `Commit` that dealt with Unicode diffs, which are now gone). These cases have all been updated to pass the correct types.	0bbc031e7434fe7949be8659ad379caa45fe7280

Diff:

Revision 3 (+276 -360)

Show changes

	reviewboard/diffviewer/commit_utils.py
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/rbgateway.py
	reviewboard/scmtools/core.py
	reviewboard/scmtools/hg.py
	reviewboard/scmtools/perforce.py
	reviewboard/scmtools/svn/__init__.py
	reviewboard/scmtools/tests/test_core.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.0.x (6abdd6a)