[Toaster] DRY RUN [Review Request] 4299 "recipes: View detailed information about a recipe"

[Damian, Alexandru]

Hi,

I have comments about the content of the patch, as well.

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.

    {% if task.outcome == Task.OUTCOME_FAILED %}class=error{% elif %} ...
{%endif%}

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:

@register.simple_tag
def task_outcome_highlight(task):
    if task.outcome == Task.OUTCOME_EXISTING:
        ret = ''
  .... etc ...

and then use it in the template:  {% task_outcome_highlight task %}

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.

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.

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.

Hope this helps,
Alex






On Mon, Jan 13, 2014 at 12:42 PM, Damian, Alexandru <
alexandru.damian at intel.com> wrote:

> Hi David,
>
> This is outstanding ! Thank you !
>
> I have some comments about the form, not the content of the patch, which
> should make future work a bit easier.
>
> - 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.
>
> - 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.
>
> You can get them like this:
>
> $ git commit                  # you commit normally to git
> $ git format-patch -n1   # you get a mailable version of the last patch
> from git;
> 0001-patch-name-as-it-appears-in-subject-line.patch     # the command will
> output the filename just written
>
> Now, there are two options to send this file.
>    - if sendmail, or any MTA for that matter, works on your machine, you
> just do:
> $ git send-email --to toaster at yoctoproject.org0001-patch-name-as-it-appears-in-subject-line.patch      # replace with the
> file name just written
>
>    - 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 :).
>
>
>
> - 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.
> The patch commit message should reference the bugzilla issue like this:
>
> [YOCTO #0000]          # replace with the real bugzilla issue number
>
>
> Hope all of this makes sense :)
>
> I'll get to review the content of the patches separately.
>
> Cheers,
> Alex
>
>
>
> On Sun, Jan 12, 2014 at 6:49 PM, Reyna, David <david.reyna at windriver.com>wrote:
>
>> 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.
>>
>> - David
>>
>> > -----Original Message-----
>> > From: Lerner, Dave
>> > Sent: Sunday, January 12, 2014 9:56 AM
>> > To: Reyna, David
>> > Cc: toaster at yoctoproject.org
>> > Subject: RE: DRY RUN [Review Request] 4299 "recipes: View detailed
>> > information about a recipe"
>> >
>> > Hi David,
>> >
>> > I notice that you pluralized the recipe.html to recipes.html.  Doesn't
>> > that break the existing convention of singular for all templates?
>> >
>> > What is your suggested naming convention for templates, singular vs
>> > plural?  If those conventions are settled on by the team, then we
>> > should apply that naming convention to the project before we create a
>> > number of other templates.
>> >
>> > What do others think of the template naming conventions?
>> > Dave
>> >
>>
>> _______________________________________________
>> toaster mailing list
>> toaster at yoctoproject.org
>> https://lists.yoctoproject.org/listinfo/toaster
>>
>
>
>
> --
> Alex Damian
> Yocto Project
> SSG / OTC
>



-- 
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20140113/65e08e35/attachment-0001.html>