Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: httptunnel - Tunnels a data stream in HTTP requests https://bugzilla.redhat.com/show_bug.cgi?id=239471 s-t-rhbugzilla@xxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |s-t-rhbugzilla@xxxxxxxxxxxxx ------- Additional Comments From s-t-rhbugzilla@xxxxxxxxxxxxx 2007-11-18 23:15 EST ------- This is not an official review; I don't think I can do that yet. Also, this is actually my first review, so I'm using it as a learning experience too. Here are my comments: The spec file at the link above doesn't match the one in the SRPM: [swarren@esk extracted]$ diff ../*spec *spec 11a12 > BuildRequires: dos2unix 54d54 < - Make sure all files are encoded with UTF-8 BuildRoot doesn't match the most preferred value, although it's OK. Most preferred is: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) See http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473 I see usage of both $RPM_BUILD_ROOT (using $ variable syntax) and %{name}. The packaging guidelines indicated a requirement for consistency of macros. I'm not sure if this implies all $ or all % type variable references, or just that macros should always be used, or ...? The following files should probably be added to %doc: DISCLAIMER HACKING NEWS TODO License: (More of an issue for upstream) Logically, it's pretty clear this is GPLv2. However, the source files simply say "see COPYING for license". Isn't GPL code supposed to have a specific format for the comments in the source files that reference COPYING. And, this specific format should state GPLv2 v.s. GPLv2+ too. I'm talking about this kind of thing: ========== This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 2, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program; if not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. */ ============================================================ In %files, the following: %defattr(-,root,root,-) relies on "make install" to set the correct permissions. I've seen reviews that requested something explicit, like this: %files %defattr(0644,root,root,0755) %doc COPYING %doc README.txt %attr(0755, root, root) /sbin/fxload %{_mandir}/*/* Is the file port/getopt1.c (and other related files) a duplicate of code in the standard getopt library? Can the application simply used standard getopt? In the files port/getopt1.c and port/getopt.h, it seems like the copyright header has been mucked with (I assume by upstream) since the fourth line of the file doesn't seem to make sense, or tie into the rest of the comment: ============================================================ /* Declarations for getopt. Copyright (C) 1989,90,91,92,93,94,96,97 Free Software Foundation, Inc. the C library, however. The master source lives in /gd/gnu/lib. NOTE: The canonical source of this file is maintained with the GNU C Library. Bugs can be reported to bug-glibc@xxxxxxxxxxxxxxxx ============================================================ File Makefile has (c) statement but no license. I don't know if that's OK. Should the RFCs in the doc directory be packaged as %doc? The Debian package admittedly doesn't do this. Do you need to add another BuildRequires to ensure that iconv is available during the build? I am not totally sure if the path references in %install are OK. Is it guaranteed where the source files will be extracted for you to modify? ... $RPM_BUILD_DIR/%{name}-%{version}/AUTHORS ... I think that's it! -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review