Code review comment for lp://staging/~pfalcon/linaro-android-build-tools/descr-update

On 2 April 2012 17:48, Paul Sokolovsky <email address hidden> wrote:
>> While I am not a fan of the pink
> Well, the idea is to get attention. At least it's not red ;-).

It kind of looks like a warning (anything in the red spectrum does),
but it isn't. It is an information box. I am not sure if there is a
common colour that is used for this sort of thing, but I think it is
safe to say that pink/red isn't it.

>> My preference would be to use a heading and define CSS for it to set
>> the "New!" bit up as a different colour. That said, it is in a big colourful
>> box - it is going to get peoples attention - so I think the new is not
>> required and I would just delete it.
>
> Well, it does use CSS to define colors. Do you mean to use styles? If so,
> the need to define styles all the time (essentially, an extra level of
> indirection) is overstated. There're cases where self-containment and
> clarity is more important, and arguably this is the case.

<div class="info_hilight"> is preferred (if my memory serves) because
that way you are defining what the element contains and you then use
CSS to tell the browser how to render it. This is hardly a case of
having to define styles all the time and it makes it much easier if
someone comes along and wants you to change the colour of the element.

> And for "New!", well, it is! I probably wouldn't add it though if we
> didn't have what we have on the frontpage. But well, let's be
> user-friendly ;-). The idea is to remove "new!" in a couple of months
> and let just the box hang. (I don't even dream of actually having
> better formatted/consisted build descriptions).

Isn't this a patch that is generating some static HTML though, so all
those HTML files will have to be modified in the future? Wouldn't it
be much easier and far more maintainable to create the page using the
Django template engine?

--
James Tunnicliffe