scott2b
commented
8 years ago Review status: 0 of 3 files reviewed at latest revision, 5 unresolved discussions.
cityhallmonitor/views.py, line 33 [r1] (raw file): Not a big deal for this commit, but generally in python we want to stick with underscore_variable_names instead of camelCaseVariableNames. See PEP8 for Python style reference: https://www.python.org/dev/peps/pep-0008/
cityhallmonitor/views.py, line 33 [r1] (raw file): I am a bit concerned about performance the len() approach to this. If this is Django ORM, I believe the len call will force evaluation of the whole queryset which partly undermines the 1000 count limit. I think a count might be better. You can try this approach and see if it is performant enough. For more details on len vs. count, see Django docs https://docs.djangoproject.com/en/dev/ref/models/querysets/#when-querysets-are-evaluated
cityhallmonitor/views.py, line 37 [r1] (raw file): same thing
_cityhallmonitor/views.py, line 59 [r1] (raw file):_ Another minor style issue: keep lines <=80 characters long
templates/search.html, line 49 [r1] (raw file): I am confused about the purpose of this UX question. Is there an option to not truncate the results? I did not see that above.
Comments from Reviewable
This PR brings in some changes for search that present timeout. Specifically, it limits the search results to 1000 documents, and adds some new UI for clarifying this truncation to the user.
This change is