RFC: New GNATS web (query-pr.cgi) interface

[Shaun Amott]

On Sun, Sep 17, 2006 at 09:04:20PM +0200, Simon L. Nielsen wrote:
> I'm mainly commenting on the code, since I think most people who have
> already commented on the visual/usability part elsewhere generally
> agree, as do I, that it's an improvement over the current script.

Thanks for taking the time to review/comment on the script!

> One thing which did bug me a lot when reading it though was the use of
> the reversed conditionals like this:
> 
> $category = undef
> 	  if ($category && $category !~ /^$valid_category$/);
> 
> I think it really breaks the flow of the code, but since some people
> like it I'm not going to object if this is presereved, just bitch a
> bit here :-).

I like this quirky syntax, personally. I concede that I may have used
it in some cases where the reverse syntax might have been better for
clarity's sake.. but, I'm happy with it as-is. Sorry. :-)

> The main thing which I think should be done before commit is use of
> cgi-style.pl.  While it might seem unnecessary now, it really is much
> nicer for the people maintaing the website if there are as few places
> as possible where we need to change if modifications are made to the
> layout.  Also, the script currently doesn't totally match the style of
> the rest of the FreeBSD.org web site, which I suspect is related to it
> not using cgi-style.pl.

Yes, the style is a vast improvement over the mess we have in place now.
But for the sake of consistency, I have switched to using cgi-style.pl.
Hopefully, some of the improvements between the old style and mine can
be merged across the site at some point. But that's another discussion.

> I have attached a patch which fixes some minor issues.  Most should be
> obvious, but:
> 
> - I moved the URL variable to the start of the script since that makes
>   it simpler to change when needed, e.g. when switching to the new
>   www.freebsd.org hopefully soon...
> - I removed $FreeBSD$ from the resulting HTML, since I don't really
>   see the pointing having it there.

Seems reasonable - I've patched the script.

> I noticed that you put Convert/UU.pm in the subdir of gnatsweb.  I
> assume this is just /usr/ports/converters/p5-Convert-UU which we can
> just install on systems which need to run the CGI?

Yes, absolutely.

> One other issue which also need to be fixed is that the script don't
> work with perl 5.0, which is still requires for a bit.

I've hacked out the modern Perl bits and pieces, it now runs on 5.0.

> The last thing I can think of now is that the script should use taint
> mode like shown below, just in case.

Indeed - I intended to add it anyway.

Shaun

-- 
Shaun Amott // PGP: 0x6B387A9A
"A foolish consistency is the hobgoblin
of little minds." - Ralph Waldo Emerson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-www/attachments/20060917/03fbc195/attachment.pgp