Return-Path: <bugzilla-daemon@mozilla.org>
X-Original-To: bhearsum@wittydomain.com
Delivered-To: bhearsum@wittydomain.com
Received: from localhost (localhost.foxybanana.com [127.0.0.1])
by puddlejumper.foxybanana.com (Postfix) with SMTP id 674961722E
for <bhearsum@wittydomain.com>; Mon, 5 Mar 2007 14:57:58 -0800 (PST)
X-Greylist: from auto-whitelisted by SQLgrey-1.7.4
Received: from recluse.mozilla.org (recluse.mozilla.org [63.245.208.164])
by puddlejumper.foxybanana.com (Postfix) with ESMTP id B4C2917211
for <bhearsum@wittydomain.com>; Mon, 5 Mar 2007 14:57:33 -0800 (PST)
Received: from recluse.mozilla.org (localhost.localdomain [127.0.0.1])
by recluse.mozilla.org (8.13.1/8.13.1) with ESMTP id l25MuDpi000717
for <bhearsum@wittydomain.com>; Mon, 5 Mar 2007 14:56:14 -0800
Received: (from apache@localhost)
by recluse.mozilla.org (8.13.1/8.13.1/Submit) id l25MuDoA000712;
Mon, 5 Mar 2007 14:56:13 -0800
Date: Mon, 5 Mar 2007 14:56:13 -0800
Message-Id: <200703052256.l25MuDoA000712@recluse.mozilla.org>
From: bugzilla-daemon@mozilla.org
To: bhearsum@wittydomain.com
Subject: [Bug 370241] Buildbot patch uploader
X-Bugzilla-Reason: CC AssignedTo Reporter
X-Bugzilla-Type: newchanged
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: mozilla.org
X-Bugzilla-Component: Build & Release
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: preed@mozilla.com
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: --
X-Bugzilla-Assigned-To: bhearsum@wittydomain.com
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Changed-Fields: Attachment #257155 Flag
In-Reply-To: <bug-370241-259016@https.bugzilla.mozilla.org/>
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
X-DSPAM-Result: Whitelisted
X-DSPAM-Processed: Mon Mar 5 14:57:58 2007
X-DSPAM-Confidence: 0.9899
X-DSPAM-Probability: 0.0000
X-DSPAM-Signature: 35,45eca076854541196912156
Do not reply to this email. You can add comments to this bug at
https://bugzilla.mozilla.org/show_bug.cgi?id=370241
J. Paul Reed [:preed] <preed@mozilla.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #257155|review?(preed@mozilla.com) |review+
Flag| |
--- Comment #22 from J. Paul Reed [:preed] <preed@mozilla.com> 2007-03-05 14:56:10 PST ---
(From update of attachment 257155)
In general, it may be useful to look over these:
https://intranet.mozilla.org/Build:PerlStyleGuide
(Most notably for captilization/indentation and function call semantics.)
># the size limit for the file, in bytes
>my $SIZELIMIT = 10*1024*1024;
Might add to the comment that this is 10 megs.
>sub write_success_page {
> print "Content-type: text/html\n\n";
> print <<ENDOFHTML;
I tend to write these as:
print <<__END_OF_HTML__; so they're easier to pick out from the code.
>sub process {
> my $time = time();
> # get the parameters
> my $name = "";
> $name = $ENV{'REMOTE_USER'};
> if ($name eq "") {
> $name = "Noname";
> }
Isn't it a requirement that people log in? Or are we just doing that for the
trial? If it's a requirement, we should error out if $name eq "".
But I can't remember which we decided.
> # only allow alphanumeric, hyphens, and single dots
> if ($patchfile !~ /^([\w-]|\.[\w-])+$/) {
> # ends the script
This is in the filename, right? Why do we allow single dots? So people can name
them patch.foo.bar?
> # write the patch
> my $filename = "$PATCH_DIR/$patchfile";
> open PATCH, ">$filename" or write_page($description,
> "Server error - Could not open file for writing.") and return;
> binmode PATCH;
We probably don't want to blindly overwrite files; there's a race-condition if
you use -e; hrm... something to think about.
For the architectural stuff, I'm mostly ok with the patch as is; I'd like to
see some of the formatting stuff fixed, though.
In general, r=preed. Good job on seeing this through, Ben!
--
Configure bugmail: https://bugzilla.mozilla.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.
You reported the bug.