Skip to content
This repository has been archived by the owner on Feb 1, 2018. It is now read-only.

implemented daily for crashstats fixes bug 788043 #145

Merged
merged 1 commit into from Oct 12, 2012

Conversation

rhelmer
Copy link
Contributor

@rhelmer rhelmer commented Oct 12, 2012

This builds on code from Schalk Neethling sneethling@mozilla.com
and Brandon Savage brandon@brandonsavage.net

@@ -23,3 +24,19 @@ def urlencode(txt):
if isinstance(txt, unicode):
txt = txt.encode('utf-8')
return urllib.quote_plus(txt)

@register.filter
def digitgroupseparator(int_):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just call it number to avoid the name clash :)

@rhelmer
Copy link
Contributor Author

rhelmer commented Oct 12, 2012

Things I am unhappy with about this patch:

  1. the way we are handling forms (doing it right will take more careful consideration, especially if we don't want to break backwards compat - this page is due for a redesign anyway)
  2. the daily view is Too Damn Long
  3. the way the data table is laid out is pretty complex, maybe this should be shoved up into the view, not sure though... I can't think of a way to transform the mware JSON to what we need for the table that isn't horrible.

Point 3 could probably be helped if I knew more about jinja templating, there are neat looping constructs in the docs that I don't quite grok yet.

<td>
<select id="daily_search_version_form_products" name="p">
{% for product_name in products %}
{% if product == product_name %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this to:

<option value="{{ product_name }}" {% if product == product_name %}selected{% endif %}
>{{ product_name }}</option>

To avoid possibly DRY.

@peterbe
Copy link
Contributor

peterbe commented Oct 12, 2012

Test coverage is a big weak but I'm not going to block because I think post-launch this is going to need to be significantly reviewed anyway (...I seem to remember from a irc chat).

r+

This builds on code from Schalk Neethling <sneethling@mozilla.com>
and Brandon Savage <brandon@brandonsavage.net>
rhelmer added a commit that referenced this pull request Oct 12, 2012
implemented daily for crashstats fixes bug 788043
@rhelmer rhelmer merged commit 6198dcc into mozilla:master Oct 12, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants