Date: Sun, 17 Sep 2006 23:32:29 +0100 From: Shaun Amott <shaun@FreeBSD.org> To: "Simon L. Nielsen" <simon@FreeBSD.org> Cc: freebsd-www@FreeBSD.org Subject: Re: RFC: New GNATS web (query-pr.cgi) interface Message-ID: <20060917223229.GA1304@picobyte.net> In-Reply-To: <20060917190419.GC33937@zaphod.nitro.dk> References: <20060912201245.GA1915@picobyte.net> <20060917190419.GC33937@zaphod.nitro.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
--C7zPtVaVf+AK4Oqc Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: >=20 > $category =3D undef > if ($category && $category !~ /^$valid_category$/); >=20 > 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: >=20 > - 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 --=20 Shaun Amott // PGP: 0x6B387A9A "A foolish consistency is the hobgoblin of little minds." - Ralph Waldo Emerson --C7zPtVaVf+AK4Oqc Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (FreeBSD) iD8DBQFFDcz9kmhdCGs4epoRAvcNAKDBq3EPyuIgz15b8FOXphUTVq3weACfQJpV o8xvko8VzvAV0+lejzmyL6w= =mnZA -----END PGP SIGNATURE----- --C7zPtVaVf+AK4Oqc--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060917223229.GA1304>