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

On Mon, 02 Apr 2012 17:31:45 -0000
James Tunnicliffe <email address hidden> wrote:

> 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.

Yes, that's the intention. What it tries to say is "Don't try to miss
me - if you do, and will read the below, your eyes will bleed because
styling there is much worse, and also you might get a headache
following all the (not always cleanly written) instructions below, and
there's still no warranty that it will work, because there's nor
warranty that those instructions are up to date".

>
> >> 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

There's no "preferred" on the global scale, there's only flexibility to
do it this or that way on that scale. It's just as saying that
"preferred" way to do an addition is to assign operands to variables
first:

a = 2; b = 2; c = a + b; c == 4

Nope, it's still fine to just do 2 + 2 = 4 when you need just that ;-).

> 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.

Yes, that works really well (and gets extra boons) when thing is well
maintained. When it is not, making a simple self-contained changes is a
better approach.

> > 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?

Actually, pragmatically, what this patch does is finishing deployment
of Andy's work is the least disturbing and the most straightforward
way. Now, semantically, it exactly implements poor man's template
engine, so it would be easy to replace styling/text of that header.
Finally, syntactically, the end product of that simple templating
engine is indeed static html multiplied across jobs. That's because we
settled on using that mechanism for maintaining dynamically changeable
job descriptions. The boon of that is that it works on Jenkins side to:
https://android-build.linaro.org/jenkins/job/pfalcon_new-instr-sample/

Finally, yes - there're other, more complicated (some maybe even
better) ways to achieve that. But as usual, we have something more
important to work on. For example, in relation to job descriptions,
it's
https://blueprints.launchpad.net/linaro-android-infrastructure/+spec/linaro-android-buildd-edit-description
We currently simply don't have any scalable way to let Android
engineers edit their job descriptions. So, we use non-scalable way of
giving full access to Jenkins. They already started to, well, abuse it
to do more changes than just job descriptions. (next step would us
getting weird problems with Jenkins, like we have on ci.*, whereas
previously we didn't have as we stick to controller subset of Jenkins
functionality).

--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog