Summary

Conditions for adding DefaultReviewers to reviewrequest

Review Request #9579 — Created Feb. 3, 2018 and discarded Sept. 14, 2021, 11:17 p.m.

Information

Owner

rodgerwa

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

Description

Replaces file_match field with a user set conditions which applied
will be tested on all posted review requests for the corresponding
repositories.

Changes:

Removed file_match field from form
Added Conditions widget to form
Added description field to form and as a list column
Added diffed file options (corresponding to old regex functionality)
Added evolution for database
Refactored code for adding default reviewers to a ReviewRequest
Added lazy migration for default reviewers
- Migrated when tested against review requests
- Migrated when before being edited by an admin

Testing Done


Created a test repository with a test struture to try various

conditions including string matching and files diffed.
Created commit to trigger migrations and be applied to review

request.
Test commit as review request to verify expected behavior 

before applying evolution
Applied evolution
Posted review request to verify same behavior was exhibited
Test other string operations
ends with
starts with
contains

Issues

Description	From	Last Updated
E122 continuation line missing indentation or outdented	reviewbot	March 7, 2018, 1:42 p.m.
E501 line too long (97 > 79 characters)	reviewbot	March 7, 2018, 1:42 p.m.
F401 'djblets.conditions.choices.BaseConditionChoice' imported but unused	reviewbot	March 7, 2018, 1:40 p.m.
F401 'reviewboard.scmtools.conditions.RepositoriesChoice' imported but unused	reviewbot	March 7, 2018, 1:40 p.m.
F401 'reviewboard.reviews.conditions.ReviewRequestConditionChoiceMixin' imported but unused	reviewbot	March 7, 2018, 1:40 p.m.
F811 redefinition of unused 'ReviewRequestRepositoryTypeChoice' from line 19	reviewbot	March 7, 2018, 1:40 p.m.
F811 redefinition of unused 'ReviewRequestRepositoriesChoice' from line 19	reviewbot	March 7, 2018, 1:40 p.m.
F811 redefinition of unused 'ReviewRequestConditionChoices' from line 19	reviewbot	March 7, 2018, 1:40 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:40 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:39 p.m.
E501 line too long (86 > 79 characters)	reviewbot	March 7, 2018, 1:39 p.m.
E501 line too long (85 > 79 characters)	reviewbot	March 7, 2018, 1:39 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:38 p.m.
E122 continuation line missing indentation or outdented	reviewbot	March 7, 2018, 1:32 p.m.
E501 line too long (97 > 79 characters)	reviewbot	March 7, 2018, 1:32 p.m.
F401 'djblets.conditions.choices.BaseConditionChoice' imported but unused	reviewbot	March 7, 2018, 1:32 p.m.
F401 'reviewboard.scmtools.conditions.RepositoriesChoice' imported but unused	reviewbot	March 7, 2018, 1:32 p.m.
F401 'reviewboard.reviews.conditions.ReviewRequestConditionChoiceMixin' imported but unused	reviewbot	March 7, 2018, 1:32 p.m.
F811 redefinition of unused 'ReviewRequestRepositoryTypeChoice' from line 19	reviewbot	March 7, 2018, 1:32 p.m.
F811 redefinition of unused 'ReviewRequestRepositoriesChoice' from line 19	reviewbot	March 7, 2018, 1:32 p.m.
F811 redefinition of unused 'ReviewRequestConditionChoices' from line 19	reviewbot	March 7, 2018, 1:32 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:32 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:32 p.m.
E501 line too long (86 > 79 characters)	reviewbot	March 7, 2018, 1:32 p.m.
E501 line too long (85 > 79 characters)	reviewbot	March 7, 2018, 1:32 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:32 p.m.
W293 blank line contains whitespace	reviewbot	March 7, 2018, 1:32 p.m.
F401 'djblets.conditions.choices.BaseConditionChoice' imported but unused	reviewbot	March 7, 2018, 1:31 p.m.
F811 redefinition of unused 'ReviewRequestRepositoryTypeChoice' from line 19	reviewbot	March 7, 2018, 1:31 p.m.
F811 redefinition of unused 'ReviewRequestRepositoriesChoice' from line 19	reviewbot	March 7, 2018, 1:31 p.m.
F811 redefinition of unused 'ReviewRequestConditionChoices' from line 19	reviewbot	March 7, 2018, 1:31 p.m.
E127 continuation line over-indented for visual indent	reviewbot	March 7, 2018, 1:31 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:31 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:31 p.m.
W293 blank line contains whitespace	reviewbot	March 7, 2018, 1:31 p.m.
F401 'djblets.conditions.Condition' imported but unused	reviewbot	March 7, 2018, 1:31 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:31 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:31 p.m.
W293 blank line contains whitespace	reviewbot	March 7, 2018, 1:31 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 1:35 p.m.
F401 'djblets.conditions.Condition' imported but unused	reviewbot	March 7, 2018, 1:37 p.m.
F401 'djblets.conditions.ConditionSet' imported but unused	reviewbot	March 7, 2018, 1:37 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 12:57 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 7, 2018, 12:57 p.m.
Please undo this change.	david	March 10, 2018, 11:29 a.m.
The comma isn't necessary here. We probably also don't need to say it replaces X fields here in the UI, …	david	March 10, 2018, 11:29 a.m.
django before djblets (alphabetical order)	david	March 10, 2018, 11:29 a.m.
Having each of the models in their own file is an implementation detail. We still want to import from reviewboard.reviews.models.	david	March 10, 2018, 11:29 a.m.
Please undo this change.	david	March 10, 2018, 11:29 a.m.
Please add a trailing comma to this line.	david	March 10, 2018, 11:29 a.m.
Can you add a module docstring to this?	david	March 10, 2018, 11:29 a.m.
Swap these two lines.	david	March 10, 2018, 11:29 a.m.
No blank line here.	david	March 10, 2018, 11:29 a.m.
No blank lines here.	david	March 10, 2018, 11:29 a.m.
Shouldn't have the blank line above this, but this should be sorted with the other reviewboard.reviews imports.	david	March 10, 2018, 11:29 a.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 9, 2018, 4:12 p.m.
Can you use single-quotes here?	david	March 10, 2018, 11:29 a.m.
Condition -> Conditions?	david	March 10, 2018, 11:29 a.m.
Blank line between these two please.	david	March 10, 2018, 11:29 a.m.
Blank line between these two please.	david	March 10, 2018, 11:29 a.m.
What was the case that was causing this?	david	March 10, 2018, 11:29 a.m.
Blank line between these two please.	david	March 10, 2018, 11:29 a.m.
No blank line here.	david	March 10, 2018, 11:29 a.m.
c before d	david	March 10, 2018, 11:29 a.m.
I know the other fields use double-quotes and lower case, but we should have new stuff use single-quotes and uppercase. …	david	March 10, 2018, 11:29 a.m.
Review Bot complained about this but indentation is wacky. How about: conditions = JSONField(default={ 'mode': ConditionSet.DEFAULT_MODE, 'conditions': [], })	david	March 10, 2018, 11:29 a.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 9, 2018, 4:12 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	March 9, 2018, 4:12 p.m.
review_request should probably be called draft because it's operating on a draft.	david	March 10, 2018, 11:29 a.m.
Docstring summaries should be in the imperative mood (Return X) rather than passive (Returns X). There's also a typo (wether) …	david	March 10, 2018, 11:29 a.m.
This should be prefixed with "Args:" and indented another 4 spaces. We're missing docs for the review_request argument (well, if …	david	March 10, 2018, 11:29 a.m.
These two conditions can be combined together: if (check_repo and review_request.repository not in self.repository.all()):	david	March 10, 2018, 11:29 a.m.
Blank line between these two.	david	March 10, 2018, 11:29 a.m.
Blank line between these two.	david	March 10, 2018, 11:29 a.m.
This looks like it probably will fit on one line.	david	March 10, 2018, 11:29 a.m.
Needs a docstring.	david	March 10, 2018, 11:29 a.m.
Needs a docstring.	david	March 10, 2018, 11:29 a.m.
Needs a docstring.	david	March 10, 2018, 11:29 a.m.
Needs a docstring.	david	March 10, 2018, 11:29 a.m.
u prefixes on strings aren't necessary (we're importing unicode_literals at the top of the file).	david	March 10, 2018, 11:29 a.m.
Trailing comma.	david	March 10, 2018, 11:29 a.m.
Trailing comma.	david	March 10, 2018, 11:29 a.m.
Trailing comma.	david	March 10, 2018, 11:29 a.m.
E261 at least two spaces before inline comment	reviewbot	April 4, 2018, 8:59 p.m.
E262 inline comment should start with '# '	reviewbot	April 4, 2018, 8:59 p.m.
E265 block comment should start with '# '	reviewbot	April 4, 2018, 8:59 p.m.
E265 block comment should start with '# '	reviewbot	April 4, 2018, 8:59 p.m.
F401 'djblets.conditions.ConditionSet' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
F401 'nose.tools.set_trace' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
F401 'pdb' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
F401 'json' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	April 4, 2018, 8:59 p.m.
F401 'nose.tools.set_trace' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	April 4, 2018, 8:59 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	April 4, 2018, 8:59 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	April 4, 2018, 8:59 p.m.
F401 'djblets.webapi.fields.ListFieldType' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
F401 'nose.tools.set_trace' imported but unused	reviewbot	April 4, 2018, 8:59 p.m.
E226 missing whitespace around arithmetic operator	reviewbot	April 4, 2018, 8:59 p.m.
F841 local variable 'ke' is assigned to but never used	reviewbot	April 4, 2018, 8:59 p.m.
E303 too many blank lines (2)	reviewbot	April 4, 2018, 8:59 p.m.
E501 line too long (97 > 79 characters)	reviewbot	April 4, 2018, 8:59 p.m.
E501 line too long (97 > 79 characters)	reviewbot	April 4, 2018, 8:59 p.m.
E501 line too long (97 > 79 characters)	reviewbot	April 4, 2018, 8:59 p.m.
E501 line too long (97 > 79 characters)	reviewbot	April 4, 2018, 8:59 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/admin/urls.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E122 continuation line missing indentation or outdented
```
reviewboard/reviews/evolutions/default_reviewers_conditions_one.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (97 > 79 characters)
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
F401 'djblets.conditions.choices.BaseConditionChoice' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
F401 'reviewboard.scmtools.conditions.RepositoriesChoice' imported but unused
```

reviewboard/reviews/forms.py (Diff revision 1)

The issue has been dropped. Show all issues

F401 'reviewboard.reviews.conditions.ReviewRequestConditionChoiceMixin' imported but unused

reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestRepositoryTypeChoice' from line 19
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestRepositoriesChoice' from line 19
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestConditionChoices' from line 19
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (86 > 79 characters)
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (85 > 79 characters)
```
reviewboard/reviews/forms.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:

JS changes

Diff:

Revision 2 (+88 -19)

Show changes

	reviewboard/admin/urls.py
	reviewboard/admin/widgets.py
	reviewboard/reviews/admin.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/default_reviewers_conditions_one.py
	reviewboard/reviews/models/default_reviewer.py
	reviewboard/templates/admin/base_site.html

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/admin/urls.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E122 continuation line missing indentation or outdented
```
reviewboard/reviews/evolutions/default_reviewers_conditions_one.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (97 > 79 characters)
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
F401 'djblets.conditions.choices.BaseConditionChoice' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
F401 'reviewboard.scmtools.conditions.RepositoriesChoice' imported but unused
```

reviewboard/reviews/forms.py (Diff revision 2)

The issue has been dropped. Show all issues

F401 'reviewboard.reviews.conditions.ReviewRequestConditionChoiceMixin' imported but unused

reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestRepositoryTypeChoice' from line 19
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestRepositoriesChoice' from line 19
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestConditionChoices' from line 19
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (86 > 79 characters)
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (85 > 79 characters)
```
reviewboard/reviews/forms.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:

progress from Fed 9th added.

Summary:

WIP CODE DROP 2: Getting closer but still not working

WIP CODE DROP: ConditionField for DefaultReviewers

Description:

~		CODE DROP 2: Getting closer but still not working
	~	Replaces 'file_match' field and the select repository field with user a
	+	set conditions which applied will be tested on all posted review
	+	requests.

~		Didn't get as much time as I would've liked to flesh this out. But I
~		got a really good idea where I need to go since I've tried a fair
~		amount of things. The big issue is getting the condition options
~		to display. Most of the other examples I found with conditions are
~		integrations which build forms differently than anything part of
~		reviewboard as far as I can tell.
	~	This update provides an evolution to add a column for a json entry
	~	that stores the condition set. Now, settings selected in the add
	~	default reviewers for the condition field will be saved and reloaded
	~	if a default reviewer is viewed from the admin panel.
	~	This also changes the labels displayed when the list view for all the
	~	default reviewers is displayed. Now, there is only the name. A
	+	description could be a possible solution for describing this
	+	condition.

Testing Done:

	+	Created a new default reviewer, viewed that the count of reviewers
	+	changed. Edited the default reviwer, reloaded it to verify that the
	+	settings changed.

Diff:

Revision 3 (+99 -10)

Show changes

	reviewboard/admin/urls.py
	reviewboard/admin/widgets.py
	reviewboard/reviews/admin.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/default_reviewers_conditions.py
	reviewboard/reviews/models/default_reviewer.py
	reviewboard/templates/admin/base_site.html

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/admin/urls.py (Diff revision 3)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/reviews/forms.py (Diff revision 3)
The issue has been dropped. Show all issues
```
F401 'djblets.conditions.choices.BaseConditionChoice' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 3)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestRepositoryTypeChoice' from line 19
```
reviewboard/reviews/forms.py (Diff revision 3)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestRepositoriesChoice' from line 19
```
reviewboard/reviews/forms.py (Diff revision 3)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'ReviewRequestConditionChoices' from line 19
```
reviewboard/reviews/forms.py (Diff revision 3)
The issue has been dropped. Show all issues
```
E127 continuation line over-indented for visual indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 3)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 3)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:

Now adds options for diffed file regex and readds the specified repository field.

Description:

~		Replaces 'file_match' field and the select repository field with user a
	~	Replaces `file_match` field with user a
		set conditions which applied will be tested on all posted review
~		requests.
	~	requests for a specified repository.

~		This update provides an evolution to add a column for a json entry
~		that stores the condition set. Now, settings selected in the add
~		default reviewers for the condition field will be saved and reloaded
~		if a default reviewer is viewed from the admin panel.
~		This also changes the labels displayed when the list view for all the
~		default reviewers is displayed. Now, there is only the name. A
~		description could be a possible solution for describing this
~		condition.
	~	Progress to far:
	~	* Add Conditions widget to form
	~	* Remove `file_match` field from form
	~	* Add evoluion for database
	~	* Add diffed file option
	~	* Add code to execute conditions on review request post
	~
	~	Work to be done:
	+	* Add migration
	+	* Tweak regex options
	+	* Add description field

Diff:

Revision 4 (+85 -5)

Show changes

	reviewboard/admin/urls.py
	reviewboard/admin/widgets.py
	reviewboard/reviews/admin.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/default_reviewers_conditions.py
	reviewboard/reviews/models/base_review_request_details.py
	2 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/admin/urls.py (Diff revision 4)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/reviews/models/base_review_request_details.py (Diff revision 4)
The issue has been dropped. Show all issues
```
F401 'djblets.conditions.Condition' imported but unused
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 4)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 4)
The issue has been dropped. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:: Grammar, bullet points, words.
Description:: Replaces file_match field with user a
set conditions which applied will be tested on all posted review
requests for a specified repository.

~
Progress to far:
~
Progress to far:
- * Add Conditions widget to form
- * Remove file_match field from form
- * Add evoluion for database
- * Add diffed file option
- * Add code to execute conditions on review request post

~
Work to be done:
~ * Add migration
~ * Tweak regex options
~ * Add description field
~

Add Conditions widget to form

~

Remove file_match field from form

~

Add evolution for database

~

Add diffed file option

+

Add code to execute conditions on review request post

+
+
Work to be done:
+
+

Add migration

+

Tweak regex options

+

Add description field

Change Summary:

Now adds migrations from file_regex to conditions, automatic application of conditions to new review request (still buggy),

Description:

		Replaces `file_match` field with user a
		set conditions which applied will be tested on all posted review
		requests for a specified repository.

		Progress to far:

		Add Conditions widget to form
		Remove `file_match` field from form
		Add evolution for database
		Add diffed file option
		Add code to execute conditions on review request post
	+	Added migrations

		Work to be done:

~		Add migration
~		Tweak regex options
	~	Add description field
	~	Create Test cases
-		Add description field

Testing Done:

		Created a new default reviewer, viewed that the count of reviewers
		changed. Edited the default reviwer, reloaded it to verify that the
		settings changed.
	+
	+	Fair amount, will update one current issue is fixed.

Commit:

fd1ed14ead060f0b38cdd0827b27d5321077660c

c072e9936a24c25f60fd6d5e5fd061fcd70ab51b

Diff:

Revision 5 (+570 -30)

Show changes

	reviewboard/admin/urls.py
	reviewboard/admin/widgets.py
	reviewboard/reviews/admin.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/conditions/__init__.py
	reviewboard/reviews/conditions/choice.py
	reviewboard/reviews/conditions/choices.py
	6 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/admin/urls.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/reviews/conditions/choices.py (Diff revision 5)
The issue has been resolved. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/base_review_request_details.py (Diff revision 5)
The issue has been resolved. Show all issues
```
F401 'djblets.conditions.Condition' imported but unused
```
reviewboard/reviews/models/base_review_request_details.py (Diff revision 5)
The issue has been resolved. Show all issues
```
F401 'djblets.conditions.ConditionSet' imported but unused
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 5)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 5)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:

Finished task so far. Now requires reviewing

Summary:

WIP CODE DROP: ConditionField for DefaultReviewers

Conditions for adding DefaultReviewers to reviewrequest

Description:

~		Replaces `file_match` field with user a
~		set conditions which applied will be tested on all posted review
~		requests for a specified repository.
	~	Replaces `file_match` field with a user set conditions which applied
	~	will be tested on all posted review requests for the corresponding
	~	repositories.

~		Progress to far:
	~	Changes:

~		Add Conditions widget to form
~		Remove `file_match` field from form
~		Add evolution for database
~		Add diffed file option
~		Add code to execute conditions on review request post
~		Added migrations
~
	~	Removed `file_match` field from form
	~	Added Conditions widget to form
	~	Added description field to form and as a list column
	~	Added diffed file options (corresponding to old regex functionality)
	~	Added evolution for database
	~	Refactored code for adding default reviewers to a ReviewRequest
	~	Added lazy migration for default reviewers Migrated when tested against review requests Migrated when before being edited by an admin
-		Work to be done:
-
-		Add description field
-		Create Test cases

Testing Done:

~		Created a new default reviewer, viewed that the count of reviewers
~		changed. Edited the default reviwer, reloaded it to verify that the
~		settings changed.
~
~		Fair amount, will update one current issue is fixed.
	~	Created a test repository with a test struture to try various conditions including string matching and files diffed.
	~	Created commit to trigger migrations and be applied to review request.
	~	Test commit as review request to verify expected behavior before applying evolution
	~	Applied evolution
	~	Posted review request to verify same behavior was exhibited
	+	Test other string operations ends with starts with contains

Commit:

c072e9936a24c25f60fd6d5e5fd061fcd70ab51b

d6e14f4f738e41e6ae6e11b852567d110c4305fb

Diff:

Revision 6 (+128 -56)

Show changes

	reviewboard/admin/widgets.py
	reviewboard/reviews/admin.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/default_reviewers_conditions.py
	reviewboard/reviews/models/base_review_request_details.py
	reviewboard/reviews/models/default_reviewer.py
	2 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```

Change Summary:: Forgot to add the student projects to the reviewers...
Groups:: students

Fix it!

reviewboard/admin/widgets.py (Diff revision 6)
An issue was opened. Show all issues
```
Please undo this change.
```

reviewboard/reviews/admin.py (Diff revision 6)

An issue was opened. Show all issues

The comma isn't necessary here. We probably also don't need to say it replaces X fields here in the UI, since those fields are no longer present (and it would be confusing to users who are looking at this for the first time). We'll talk about the change in the release notes.

reviewboard/reviews/conditions.py (Diff revision 6)
An issue was opened. Show all issues
```
django before djblets (alphabetical order)
```

reviewboard/reviews/conditions.py (Diff revision 6)

An issue was opened. Show all issues

Having each of the models in their own file is an implementation detail. We still want to import from reviewboard.reviews.models.

reviewboard/reviews/conditions.py (Diff revision 6)
An issue was opened. Show all issues
```
Please undo this change.
```
reviewboard/reviews/conditions.py (Diff revision 6)
An issue was opened. Show all issues
```
Please add a trailing comma to this line.
```
reviewboard/reviews/evolutions/default_reviewers_conditions.py (Diff revision 6)
An issue was opened. Show all issues
```
Can you add a module docstring to this?
```
reviewboard/reviews/evolutions/default_reviewers_conditions.py (Diff revision 6)
An issue was opened. Show all issues
```
Swap these two lines.
```
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
No blank line here.
```
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
No blank lines here.
```

reviewboard/reviews/forms.py (Diff revision 6)

An issue was opened. Show all issues

Shouldn't have the blank line above this, but this should be sorted with the other reviewboard.reviews imports.

reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
Can you use single-quotes here?
```
1. rpolyano March 18, 2018, 9:54 a.m.
  How do you determine the type of quote to use (single vs double)?
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
Condition -> Conditions?
```
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
Blank line between these two please.
```
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
Blank line between these two please.
```
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
What was the case that was causing this?
```
reviewboard/reviews/forms.py (Diff revision 6)
An issue was opened. Show all issues
```
Blank line between these two please.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
No blank line here.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
c before d
```

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

An issue was opened. Show all issues

I know the other fields use double-quotes and lower case, but we should have new stuff use single-quotes and uppercase.

(the names all get title-cased by the UI display anyway--we'll fix up the older code at some point in the future)

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

An issue was opened. Show all issues

Review Bot complained about this but indentation is wacky. How about:

conditions = JSONField(default={
    'mode': ConditionSet.DEFAULT_MODE,
    'conditions': [],
})

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
review_request should probably be called draft because it's operating on a draft.
```

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

An issue was opened. Show all issues

Docstring summaries should be in the imperative mood (Return X) rather than passive (Returns X).
There's also a typo (wether) and some weird grammar (this requests). How about:
Return whether this default reviewer rule matches the draft.

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

An issue was opened. Show all issues

This should be prefixed with "Args:" and indented another 4 spaces. We're missing docs for the review_request argument (well, if you make the above change, draft).
We also need a "Returns:" section explaining the type and meaning of the return value.

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

An issue was opened. Show all issues

These two conditions can be combined together:

if (check_repo and
    review_request.repository not in self.repository.all()):

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Blank line between these two.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Blank line between these two.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
This looks like it probably will fit on one line.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Needs a docstring.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Needs a docstring.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Needs a docstring.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Needs a docstring.
```

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

An issue was opened. Show all issues

u prefixes on strings aren't necessary (we're importing unicode_literals at the top of the file).

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Trailing comma.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Trailing comma.
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 6)
An issue was opened. Show all issues
```
Trailing comma.
```

reviewboard/reviews/models/default_reviewer.py (Diff revision 6)

I'm guessing this is a Python convention or something but why do some methods start with a letter and some start with an '_'?

david March 14, 2018, 2:52 p.m.

Python doesn't have access control (like private/public/protected), so a leading underscore is a convention to say "this is internal and if you rely on it your code may break at any point".

Change Summary:

Messing with pdb to find out the issue with the test cases. WIP

Commit:

d6e14f4f738e41e6ae6e11b852567d110c4305fb

3fd535ff50c16c2e2e499b29af9c53b9fe41f968

Diff:

Revision 7 (+236 -68)

Show changes

	reviewboard/reviews/admin.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/default_reviewers_conditions.py
	reviewboard/reviews/models/base_review_request_details.py
	reviewboard/reviews/models/default_reviewer.py
	reviewboard/reviews/models/review_request.py
	4 more

Checks run (2 timed out)

flake8 timed out.

JSHint timed out.

Change Summary:

Current progress on moving repositories to conditions.

Commit:

3fd535ff50c16c2e2e499b29af9c53b9fe41f968

a8dbfd785d5005411962ed5cee5976f55cb4703a

Diff:

Revision 8 (+851 -175)

Show changes

	reviewboard/reviews/admin.py
	reviewboard/reviews/conditions.py
	reviewboard/reviews/forms.py
	reviewboard/reviews/evolutions/__init__.py
	reviewboard/reviews/evolutions/default_reviewers_conditions.py
	reviewboard/reviews/models/base_review_request_details.py
	reviewboard/reviews/models/default_reviewer.py
	reviewboard/reviews/models/review_request.py
	6 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/admin.py (Diff revision 8)
An issue was opened. Show all issues
```
E261 at least two spaces before inline comment
```
reviewboard/reviews/admin.py (Diff revision 8)
An issue was opened. Show all issues
```
E262 inline comment should start with '# '
```
reviewboard/reviews/admin.py (Diff revision 8)
An issue was opened. Show all issues
```
E265 block comment should start with '# '
```
reviewboard/reviews/admin.py (Diff revision 8)
An issue was opened. Show all issues
```
E265 block comment should start with '# '
```
reviewboard/reviews/forms.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'djblets.conditions.ConditionSet' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'nose.tools.set_trace' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'pdb' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'json' imported but unused
```
reviewboard/reviews/forms.py (Diff revision 8)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'nose.tools.set_trace' imported but unused
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/reviews/models/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E126 continuation line over-indented for hanging indent
```
reviewboard/webapi/resources/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'djblets.webapi.fields.ListFieldType' imported but unused
```
reviewboard/webapi/resources/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
F401 'nose.tools.set_trace' imported but unused
```
reviewboard/webapi/resources/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E226 missing whitespace around arithmetic operator
```
reviewboard/webapi/resources/default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
F841 local variable 'ke' is assigned to but never used
```
reviewboard/webapi/tests/test_default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E303 too many blank lines (2)
```
reviewboard/webapi/tests/test_default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E501 line too long (97 > 79 characters)
```
reviewboard/webapi/tests/test_default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E501 line too long (97 > 79 characters)
```
reviewboard/webapi/tests/test_default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E501 line too long (97 > 79 characters)
```
reviewboard/webapi/tests/test_default_reviewer.py (Diff revision 8)
An issue was opened. Show all issues
```
E501 line too long (97 > 79 characters)
```

Status:: Discarded