Add a model representation of a commit in a series
Review Request #9609 — Created Feb. 9, 2018 and submitted
This is the first of a series of commits to introduce support for commit
series review. This patch adds the necessary database model
(DiffCommit) and updates the existing models to introduce
relationships.
Ran unit tests.
Applied evolution.
| Description | From | Last Updated |
|---|---|---|
|
F821 undefined name 'text' |
 reviewbot
|
|
|
E303 too many blank lines (2) |
reviewbot
|
|
|
W292 no newline at end of file |
reviewbot
|
|
|
Git commits currently use a 40-character hash, but I'm not sure that's future proof, or even current proof for other … |
 david
|
|
|
E303 too many blank lines (4) |
reviewbot
|
|
|
E303 too many blank lines (2) |
reviewbot
|
|
|
Two blank lines here. |
 chipx86
|
|
|
Can you add doc comments for any of these meant to be public, and prefix any that aren't with a … |
chipx86
|
|
|
This might be useful to put in a validators.py. |
chipx86
|
|
|
These should also have blank=True. |
chipx86
|
|
|
This should describe how the result would look, since it differs based on provided data (and may be None). |
chipx86
|
|
|
Same here. |
chipx86
|
|
|
Probably worth mentioning in this about the truncation. ... do we want truncation? I almost feel like if we do, … |
chipx86
|
|
|
This doesn't mention that it may be None. |
chipx86
|
|
|
Worth mentioning, email.utils.formataddr has basically the same logic, with the difference being that an empty e-mail will be in the … |
chipx86
|
|
|
F401 'reviewboard.diffviewer.validators.COMMIT_ID_RE' imported but unused |
reviewbot
|
|
|
Needs to be one line. These don't need (and probably shouldn't have) literals in the summary. Just say "The maximum … |
chipx86
|
|
|
Now that we have summary_truncated, how about getting rid of truncation from summary? |
chipx86
|
|
|
I see the need to split this out, but I'm wondering whether this is the right name. How about something … |
chipx86
|
|
|
Backtick should be removed. |
chipx86
|
|
|
This should specify when it might be None. |
chipx86
|
|
|
Missing a docstring. |
chipx86
|
|
|
Do we want > 80 or >= 80? |
chipx86
|
|
|
This won't link anymore. Needs to be the full path. |
chipx86
|
|
|
Why's this removed? |
chipx86
|
|
|
Kinda feel like this really should be "commit_count." We also have a "commit" field, and a "file_count," and we should … |
chipx86
|
|
|
Should be single quotes. |
chipx86
|
|
|
Too many "f"s in "mixin". Also typo in "collection." |
chipx86
|
- Change Summary:
-
pep8
- Commit:
-
7da93162016cfef0ef5c94de1168ca3f31e46d8c49b4d58b7bbd7bba14bee778af0a1fcf7e8ea284
- Diff:
-
Revision 2 (+247 -4)
Checks run (2 succeeded)
- Change Summary:
-
Refactored out
get_total_line_countsinto a mixin for inclusion inDiffCommit. - Commit:
-
49b4d58b7bbd7bba14bee778af0a1fcf7e8ea284e8da97413e8ccd7f8e56425b50d65ddb757b4d45
- Diff:
-
Revision 3 (+287 -31)
- Commit:
-
e8da97413e8ccd7f8e56425b50d65ddb757b4d458a8fb4463d44c23c5813f87c71e1ed9f9b1b2714
- Diff:
-
Revision 4 (+287 -31)
Checks run (2 succeeded)
- Commit:
-
8a8fb4463d44c23c5813f87c71e1ed9f9b1b271409698fb76131e56cfde691c05b88d1654a78034a
- Diff:
-
Revision 5 (+287 -31)
Checks run (2 succeeded)
- Commit:
-
09698fb76131e56cfde691c05b88d1654a78034af10c8ff324aacd9e54df5392dd187db0724a36fd
- Diff:
-
Revision 6 (+288 -31)
- Commit:
-
f10c8ff324aacd9e54df5392dd187db0724a36fd6c8036075c2ba280a1f9a83877c118bef4acb5d5
- Diff:
-
Revision 7 (+286 -31)
Checks run (2 succeeded)
- Commit:
-
6c8036075c2ba280a1f9a83877c118bef4acb5d570a9d07bcc97251a272617cdb25b488fa7263b11
- Diff:
-
Revision 8 (+287 -31)
- Commit:
-
70a9d07bcc97251a272617cdb25b488fa7263b11dbf1718923c96c480d4251466ee492234c7547f9
- Diff:
-
Revision 9 (+286 -31)
Checks run (2 succeeded)
-
-
-
-
-
-
This should describe how the result would look, since it differs based on provided data (and may be
None). -
-
Probably worth mentioning in this about the truncation.
... do we want truncation? I almost feel like if we do, it should be its own property.
-
-
Worth mentioning,
email.utils.formataddrhas basically the same logic, with the difference being that an empty e-mail will be in the form ofname <>. Still, maybe we want to use that instead.
- Change Summary:
-
Addressed open issues.
- Commit:
-
dbf1718923c96c480d4251466ee492234c7547f9dcbc9be2f8e2921fdaffebc27958fe581efbd2ab
- Diff:
-
Revision 10 (+309 -31)
- Commit:
-
dcbc9be2f8e2921fdaffebc27958fe581efbd2ab762d931214eb14e137b379c7921bf52382d0c469
- Diff:
-
Revision 11 (+328 -31)
Checks run (2 succeeded)
-
-
Needs to be one line.
These don't need (and probably shouldn't have) literals in the summary. Just say "The maximum length of the e-mail address fields" here, and similar above.
-
-
I see the need to split this out, but I'm wondering whether this is the right name. How about something like
FileDiffCollectionMixin? We're really talking about something that provides operations for objects that manage filediffs. I could see us expanding this down the road.In fact, since we're modifying
DiffSetanyway, one immediately useful thing I can think of is aRelationCounterFieldfor the files, so that logic doesn't have to query for file countds.
- Commit:
-
762d931214eb14e137b379c7921bf52382d0c469c93b01fa150880c6dcc28997ccff456dcbeb1909
- Diff:
-
Revision 12 (+334 -32)
Checks run (2 succeeded)
- Change Summary:
-
Fix unit test failures.
- Commit:
-
c93b01fa150880c6dcc28997ccff456dcbeb1909ad382200a5608b9a7a29f9b1139ea7b92bb789f1
- Diff:
-
Revision 13 (+334 -32)
Checks run (2 timed out)
- Change Summary:
-
Actually commit changes.
- Commit:
-
ad382200a5608b9a7a29f9b1139ea7b92bb789f10793769fdf4b64833749be7fdb1653638dd74097
- Diff:
-
Revision 14 (+334 -33)
Checks run (2 timed out)
- Change Summary:
-
Addressed Christian's feedback
- Commit:
-
0793769fdf4b64833749be7fdb1653638dd740970a9cad7a7a516a8174f0457de3c4cebb665c641f
- Diff:
-
Revision 15 (+339 -33)
Checks run (2 succeeded)
- Change Summary:
-
Rename description -> commit_message from feedback on /r/9642/
- Commit:
-
0a9cad7a7a516a8174f0457de3c4cebb665c641fda3a490690f9349245f3d23a6d2415325d8a96c3
- Diff:
-
Revision 16 (+339 -33)
Checks run (2 succeeded)
- Change Summary:
-
In preparation for a change that re-standardizes us on
diffsetoverdiff_set. - Commit:
-
da3a490690f9349245f3d23a6d2415325d8a96c3fdab7f009dedd90b1b96f0edb4094218b613c4e9
- Diff:
-
Revision 17 (+339 -33)
Checks run (2 succeeded)
- Change Summary:
-
Addressed Christian's feedback.
- Commit:
-
fdab7f009dedd90b1b96f0edb4094218b613c4e9a1bf88f040dd04501510293633a7d2c185962bce
- Diff:
-
Revision 18 (+339 -33)
Checks run (2 succeeded)
- Depends On:
-
- Commit:
a1bf88f040dd04501510293633a7d2c185962bced37275e4f49e1381888d149bfc6ae9119f6e2db1- Diff:
Revision 19 (+326 -33)
Checks run (2 succeeded)
flake8 passed.JSHint passed.
- Change Summary:
-
Re-add missing evolution
- Commit:
-
d37275e4f49e1381888d149bfc6ae9119f6e2db1b8f56a6c01fa274cf6010cea29262558b875b784
- Diff:
-
Revision 20 (+340 -33)