Code review comment for lp://staging/~mwhudson/lava-scheduler/email-on-health-check-failure

On Thu, 05 Apr 2012 00:49:18 -0000, Zygmunt Krynicki <email address hidden> wrote:
> Review: Approve
>
> 149 + if len(description) > 80:
> 150 + description = '...' + description[-78:]
>
> Why the last 78 characters? Note that will make the full line 81 + newline characters long

Just a bit arbitrary really. Can bump up the limit to say 200, which
would allow every description in the database today, but avoid complete
silliness.

> 112 + 'lava_scheduler_app/job_summary_mail.txt',
> 113 + {
> 114 + 'job': self,
> 115 + })
>
> Maybe you can fit that on one line?

Yep.

> 178 +{% for run in job.results_bundle.test_runs.all %} | {{ run.test.test_id|ljust:20 }} | {{ run.get_summary_results.pass|default:0|rjust:6 }} | {{ run.get_summary_results.total|default:0|rjust:6 }} |
>
> On the note that this makes the template uglier. Recent django has
> variables and other goodies in the templates and I'm sure you can
> rewrite that to be readable if you want.

Ah, you mean {% with %} and so on? It helps a bit, but having to have
everything on the one line is a bit painful. I've improved this a bit,
am happy to take any more suggestions :-)

I also need to make sure we get full URLs in the emails, currently
they're just the /scheduler/job/16280 sort of thing.

Cheers,
mwh