Summary

Improve CommitListField HTML rendered in changedescs

Review Request #10522 — Created April 9, 2019 and updated April 22, 2019, 3:28 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

The new template supports rendering commit modifications, however
commit_utils.diff_histories presently cannot generate history diffs
that contain modifications (only additions, deletions, and unmodified
entries).

Testing Done

Ran unit tests.

Commits

Summary	ID
Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. The new template supports rendering commit modifications, however `commit_utils.diff_histories` presently cannot generate history diffs that contain modifications (only additions, deletions, and unmodified entries). Testing done: Ran unit tests.	1d5306bd97fe94ee92cb442c2d35d09ad8928f67

Summary

Improve CommitListField HTML rendered in changedescs

The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. The new template supports rendering commit modifications, however `commit_utils.diff_histories` presently cannot generate history diffs that contain modifications (only additions, deletions, and unmodified entries). Testing done: Ran unit tests.

1d5306bd97fe94ee92cb442c2d35d09ad8928f67

Files

Issues

Description	From	Last Updated
Can you add screenshots?	chipx86	April 13, 2019, 2:53 p.m.
F841 local variable 'to_expand' is assigned to but never used	reviewbot	April 9, 2019, 5:56 p.m.
F841 local variable 'to_expand' is assigned to but never used	reviewbot	April 9, 2019, 5:56 p.m.
Instead of defining yet another color constant for me to replace, how about just defining the background color for .changed …	david	April 22, 2019, 3:28 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/builtin_fields.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F841 local variable 'to_expand' is assigned to but never used
```

Commits:

	Summary	ID
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: - Ran unit tests.	cc056f539b473aa24400a99e6f5b13a0b58be6bf
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: Ran unit tests.	05b9937d190618fd8d37f896bfe02a2f15bfd95c

Diff:

Revision 2 (+462 -102)

Show changes

	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/tests/test_builtin_fields.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/templates/reviews/changedesc_commit_list.html

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/builtin_fields.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F841 local variable 'to_expand' is assigned to but never used
```

Testing Done:

~		Ran unit tests.
	~	Ran unit tests.

Commits:

	Summary	ID
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: Ran unit tests.	05b9937d190618fd8d37f896bfe02a2f15bfd95c
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: Ran unit tests.	33d1f94cf6070af08b32eaf1cfcdea24372efa3b

Diff:

Revision 3 (+458 -102)

Show changes

	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/tests/test_builtin_fields.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/templates/reviews/changedesc_commit_list.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

attach unit tests

Commits:

	Summary	ID
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: Ran unit tests.	33d1f94cf6070af08b32eaf1cfcdea24372efa3b
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: Ran unit tests.	6b417754f3bbcb0af7a19619e0060cae5d8a19b6

Diff:

Revision 4 (+456 -102)

Show changes

	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/tests/test_builtin_fields.py
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/templates/reviews/changedesc_commit_list.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues
```
Can you add screenshots?
```

Change Summary:

Added screenshots.

NOTE:

In the "Modifying a single commit (after)" screenshot, diff_histories cannot detect commit modifications, so this is a screenshot of what it would look like if it could.

Description:

		The CommitListField now renders change description entries for the field
		more similarily to its regular field view. Previously, it would just
		show all old commits as removed and all new commits as added, but now it
		performs a history diff between the old and new commits and renders that
		instead.
	+
	+	The new template supports rendering commit modifications, however
	+	`commit_utils.diff_histories` presently cannot generate history diffs
	+	that contain modifications (only additions, deletions, and unmodified
	+	entries).

Added Files:

Commits:

	Summary	ID
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. Testing done: Ran unit tests.	6b417754f3bbcb0af7a19619e0060cae5d8a19b6
	Improve CommitListField HTML rendered in changedescs The CommitListField now renders change description entries for the field more similarily to its regular field view. Previously, it would just show all old commits as removed and all new commits as added, but now it performs a history diff between the old and new commits and renders that instead. The new template supports rendering commit modifications, however `commit_utils.diff_histories` presently cannot generate history diffs that contain modifications (only additions, deletions, and unmodified entries). Testing done: Ran unit tests.	1d5306bd97fe94ee92cb442c2d35d09ad8928f67

Diff:

Revision 5 (+472 -104)

Show changes

	reviewboard/reviews/builtin_fields.py
	reviewboard/reviews/tests/test_builtin_fields.py
	reviewboard/static/rb/css/defs.less
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/templates/reviews/changedesc_commit_list.html

Removed Files:

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Fix it!

reviewboard/static/rb/css/defs.less (Diff revision 5)

An issue was opened. Show all issues

Instead of defining yet another color constant for me to replace, how about just defining the background color for .changed td as a variable and then using darken(..., ...)?