<div dir="ltr"><div><div><div><div><div><div><div><div><div><div><div><div>Hi,<br><br>I have comments about the content of the patch, as well.<br><br></div>a). models.py; I am not very fond of having display code in the models, because it breaks the MVC separation. The display code should live in the templates, e.g.<br>
</div><br>Â Â Â {% if task.outcome == Task.OUTCOME_FAILED %}class=error{% elif %} ... {%endif%}<br><br></div>If this is repeated elsewhere, like in this particular case, you should define a new tag in ./bitbake/lib/toaster/toastergui/templatetags/projecttags.py that returns the code:<br>
<br></div>@register.simple_tag<br></div>def task_outcome_highlight(task):<br></div>Â Â Â if task.outcome == Task.OUTCOME_EXISTING:<br></div>Â Â Â Â Â Â Â ret = ''<br></div>Â .... etc ...<br><br></div>and then use it in the template:Â {% task_outcome_highlight task %}<br>
<br></div><div>b). html format; please don't use tabs; use 4-space instead of a tab; this makes for a nice consistency with the HTML code, and fits the HTML code in page better.<br></div><div><br></div>c). recipe.html; please extend "basebuildpage.html" which fills in the breadcrumb and all the layout for viewing pages in a build context. see configvars.html for an example.<br>
<br></div>d). recipes.html; the latest versions of the basetable_top will automatically generate correct table header based on the context description; please don't include your own table header, but edit the page context to correctly set the table data; see the build view / build.html as a how-to guide.<br>
<br></div>Hope this helps,<br>Alex<br><div><br><div><br><div><br><div><div><div><div><div><div><div><br></div></div></div></div></div></div></div></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Mon, Jan 13, 2014 at 12:42 PM, Damian, Alexandru <span dir="ltr"><<a href="mailto:alexandru.damian@intel.com" target="_blank">alexandru.damian@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div><div><div><div><div><div><div><div><div>Hi David,<br><br>This is outstanding ! Thank you !<br><br></div>I have some comments about the form, not the content of the patch, which should make future work a bit easier.<br>
<br></div>- Indeed, we use the pluralized name for pages showing an object list (i.e. tables), and the singular for the detail page for the a single object.<br>
</div><br>- We can do patches over email, instead of git push, but it's harder to work this way. However, the email patches should be proper git patches for easy use.<br></div><br>You can get them like this:<br></div>
<br>$ git commit                 # you commit normally to git<br></div>$ git format-patch -n1  # you get a mailable version of the last patch from git; <br></div>0001-patch-name-as-it-appears-in-subject-line.patch    # the command will output the filename just written<br>
<br></div>Now, there are two options to send this file.<br></div>  - if sendmail, or any MTA for that matter, works on your machine, you just do:<br><div>$ git send-email --to <a href="mailto:toaster@yoctoproject.org" target="_blank">toaster@yoctoproject.org</a> 0001-patch-name-as-it-appears-in-subject-line.patch     # replace with the file name just written<br>
<br></div><div>Â Â - otherwise, open up your mail client, and copy/paste the contents of the file written in the mail, starting line 6, or just after the empty line after the Subject: line. Copy/paste the Subject: line content as your Subject:. Send this email :).<br>
</div><div><div><div><br><br><br></div><div>- The test procedure is outstanding ! I think we should use it for all the patches. For easier reference, I propose to get the test procedure in the bugzilla item that the patch addresses, as a comment.<br>
</div><div>The patch commit message should reference the bugzilla issue like this:<br><br></div><div>[YOCTO #0000]Â Â Â Â Â Â Â Â Â # replace with the real bugzilla issue number<br></div><div><br><br></div><div>Hope all of this makes sense :)<br>
</div><div><br></div><div>I'll get to review the content of the patches separately. <br><br></div><div>Cheers,<br>Alex<br></div><div><br></div></div></div></div><div class="gmail_extra"><div><div class="h5"><br><br><div class="gmail_quote">
On Sun, Jan 12, 2014 at 6:49 PM, Reyna, David <span dir="ltr"><<a href="mailto:david.reyna@windriver.com" target="_blank">david.reyna@windriver.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I used "recipes.html" for the page listing all recipes, and "recipe.html" for the individual recipe detail page. I can use different naming of course.<br>
<span><font color="#888888"><br>
- David<br>
</font></span><div><br>
> -----Original Message-----<br>
> From: Lerner, Dave<br>
> Sent: Sunday, January 12, 2014 9:56 AM<br>
> To: Reyna, David<br>
> Cc: <a href="mailto:toaster@yoctoproject.org" target="_blank">toaster@yoctoproject.org</a><br>
> Subject: RE: DRY RUN [Review Request] 4299 "recipes: View detailed<br>
> information about a recipe"<br>
><br>
</div><div>> Hi David,<br>
><br>
> I notice that you pluralized the recipe.html to recipes.html. Â Doesn't<br>
> that break the existing convention of singular for all templates?<br>
><br>
> What is your suggested naming convention for templates, singular vs<br>
> plural? Â If those conventions are settled on by the team, then we<br>
> should apply that naming convention to the project before we create a<br>
> number of other templates.<br>
><br>
> What do others think of the template naming conventions?<br>
> Dave<br>
><br>
<br>
</div><div><div>_______________________________________________<br>
toaster mailing list<br>
<a href="mailto:toaster@yoctoproject.org" target="_blank">toaster@yoctoproject.org</a><br>
<a href="https://lists.yoctoproject.org/listinfo/toaster" target="_blank">https://lists.yoctoproject.org/listinfo/toaster</a><br>
</div></div></blockquote></div><br><br clear="all"><br></div></div><span class="HOEnZb"><font color="#888888">-- <br><div dir="ltr">Alex Damian<div>Yocto Project<br></div><div>SSG / OTCÂ </div></div>
</font></span></div>
</blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr">Alex Damian<div>Yocto Project<br></div><div>SSG / OTCÂ </div></div>
</div></div>