Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=292121
Summary: Review Request: boxes - Draw any kind of box around some given text Product: Fedora Version: devel Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: jhrozek@redhat.com QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://hrozkovi.cz/boxes.spec SRPM URL: http://hrozkovi.cz/boxes-1.1-1.fc8.src.rpm
Description: "boxes" can draw all kinds of boxes around its input text, ranging from a C comment box to complex ASCII art. These boxes may also be removed, even if they have been badly damaged by editing of the text inside. Since boxes may be open on any side, "boxes" can also be used to create regional comments in any programming language. With the help of an editor macro or mapping, damaged boxes can easily be repaired. New box designs of all sorts can easily be added and shared by appending to a free format configuration file.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From thomas.moschny@gmx.de 2007-09-18 15:17 EST ------- Note: This is NOT a formal review (as I am not a reviewer).
[x] package meets naming guidelines [x] specfile is encoded in ascii or utf-8 [x] specfile matches base package name [!] specfile uses macros consistently - use %{_bindir} in %install - maybe use a macro for the value of GLOBALCONF, because this is used twice [x] specfile is written cleanly [!] specfile is written in AE contains non-english comments [x] changelog is present and has correct format [x] license matches actual license [x] license is open source-compatible [x] license text is included in package [x] source tag has correct url [x] source files match upstream md5sum: d2ef9fa28a87bf32b3fe0c47ab82fa97 [x] latest version is packaged [x] summary is concise [x] dist tag is present [x] buildroot is correct [x] buildroot is prepped [x] %clean is present [x] proper build requirements [x] proper requirements [!] uses %{?_smp_mflags} [!] uses %{optflags} possibly requires patching the Makefile [x] doesn't use %makeinstall [x] package builds at least on one architecture tested on: fc7 x86_64 [!] packages installs and runs at least on one architecture boxes fails on fc7 x86_64 with: boxes: Alleged system-wide config file '/usr/share/boxes' is a directory [!] rpmlint is quiet boxes.src:26: W: setup-not-quiet boxes.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 8) [x] final provides/requires look sane [-] ldconfig called in %post and %postun if required [x] code, not content [x] file permissions are appropriate [!] debuginfo package looks usable the debuginfo package is empty, because -g is missing in CFLAGS, see above [-] config files marked as %config(noreplace) [x] owns all directories it creates [-] static libraries in -devel subpackage [-] header files in -devel subpackage [-] development .so files in -devel subpackage [-] pkgconfig files in -devel subpackage, requires pkgconfig [x] no .la files [x] doesn't need a -docs subpackage [x] relevant docs are included [x] doc files are not needed at runtime [-] provides a .desktop file, build-requires desktop-file-utils [-] uses %find_lang, build-requires gettext
Additional notes / suggestions:
- consider packaging doc/boxes.el (to ${datadir}/%{name}) - use install -p to preserve timestamps - default permissions for directories are not set in %defattr - use wildcard: %{_mandir}/man1/boxes.1*
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From jhrozek@redhat.com 2007-09-30 14:44 EST ------- Thomas, thanks for taking a look at my package. Indeed, most of the things you pointed out were packaging mistakes and are fixed at:
http://hrozkovi.cz/boxes.spec http://hrozkovi.cz/boxes-1.1-2.fc7.src.rpm
The only thing I could not reproduce was the error message " Alleged system-wide config file '/usr/share/boxes' is a directory" .. could you please retry? I've tested basic functionality of boxes on F7 i386 and rawhide x86_64 and on both arches it runs fine...
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From jhrozek@redhat.com 2007-09-30 14:49 EST ------- One final note, the basic functionality test in my case was: $ echo "hello" | boxes -d parchment to make sure that boxes run and are able to display a non-default box
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From packages@amiga-hardware.com 2007-09-30 18:55 EST ------- As boxes installs an elisp (.el) file, shouldn't it be packaged according to...
http://fedoraproject.org/wiki/PackagingDrafts/EmacsenAddOns?highlight=%28ema...
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From thomas.moschny@gmx.de 2007-10-02 05:42 EST ------- (In reply to comment #2)
Most issues are fixed. In line 45, you should replace '%{_prefix}/bin' with '%{_bindir}'.
Some minor remarks: - There's a full stop missing after the %description. - No need to put a '/' before %{_prefix} or similar paths. - Default permissions for directories are not set in %defattr (fourth arg). - The patch file contains a whitespace change. - The patch file is named 'fix_smp_flags' but also comments the strip command from the Makefile.
The only thing I could not reproduce was the error message " Alleged system-wide config file '/usr/share/boxes' is a directory" .. could you please retry? I've tested basic functionality of boxes on F7 i386 and rawhide x86_64 and on both arches it runs fine...
Using your latest specfile, the package runs fine on F7 i386 and F7 x86_64.
(The problem lies in the Makefile: The sed command to patch src/boxes.h is only executed if there's something newer than src/boxes.h. So, changing GLOBALCONF via the make commandline only doesn't have any effect unless that sed command is forced to be executed, e.g. by touching or patching the Makefile, as you do now.)
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From thomas.moschny@gmx.de 2007-10-02 06:03 EST ------- (In reply to comment #4)
As boxes installs an elisp (.el) file, shouldn't it be packaged according
to...
http://fedoraproject.org/wiki/PackagingDrafts/EmacsenAddOns?highlight=%28ema...
You mean this section: http://fedoraproject.org/wiki/Packaging/Emacs#head-9ca98ccf54aba75a0bbda73be...
Following this would indeed require creating two subpackages, named boxes-emacs and boxes-emacs-el.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From jhrozek@redhat.com 2007-10-13 16:37 EST ------- New packages:
http://hrozkovi.cz/boxes.spec http://hrozkovi.cz/boxes-1.1-3.fc7.src.rpm
I tried to address the issues pointed out in comment #5, hope everything is packaged according to the Guidelines now..
Some minor remarks:
- There's a full stop missing after the %description.
- No need to put a '/' before %{_prefix} or similar paths.
- Default permissions for directories are not set in %defattr (fourth arg).
- The patch file contains a whitespace change.
The above should be fixed.
- The patch file is named 'fix_smp_flags' but also comments the strip
command from the Makefile. I've renamed the patch and added a comment to the specfile describing what it really does.
After some thinking, I decided that it would be wisest to drop the Emacs helper for now. I don't use Emacs myself, so I don't understand half of the terms used in the Emacs guidelines and frankly, I'm not all that much willing to learn how Emacs and its add-ons work, not to mention lisp. Maintaining something you don't use is bad.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From thomas.moschny@gmx.de 2007-11-06 11:19 EST ------- (In reply to comment #7)
I tried to address the issues pointed out in comment #5, hope everything is packaged according to the Guidelines now..
Looks good to me.
After some thinking, I decided that it would be wisest to drop the Emacs helper for now. I don't use Emacs myself, so I don't understand half of the terms used in the Emacs guidelines and frankly, I'm not all that much
willing
to learn how Emacs and its add-ons work, not to mention lisp. Maintaining something you don't use is bad.
Fair enough!
As far as I understand, the aforementioned packaging guidelines don't allow this, but: some packages install single .el files as part of the %doc's. This way, the user is freed at least from downloading the whole package himself in order to get the latest version of that emacs file.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Version|devel |rawhide
------- Additional Comments From thomas.moschny@gmx.de 2007-12-30 12:13 EST ------- At this point, an 'official' reviewer should have a look at the package, I suppose.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
wolfy@nobugconsulting.ro changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |wolfy@nobugconsulting.ro Status|NEW |ASSIGNED Flag| |fedora-review?
------- Additional Comments From wolfy@nobugconsulting.ro 2007-12-30 20:09 EST ------- I;l try to review this. Technically everything sees fine, but I have a problem with the license[s]: - upstream's website and the README included in the tarball claim GPLv2 - almost all sources are GPLv2+ - some sources (2 or three) include the following construction: * Copyright (c) 1986 by University of Toronto. * Written by Henry Spencer. Not derived from licensed software. * * Permission is granted to anyone to use this software for any * purpose on any computer system, and to redistribute it freely, * subject to the following restrictions: * * 1. The author is not responsible for the consequences of use of * this software, no matter how awful, even if they arise * from defects in it. * * 2. The origin of this software must not be misrepresented, either * by explicit claim or by omission. * * 3. Altered versions must be plainly marked as such, and must not * be misrepresented as being the original software. *
This looks like a variant of MIT to me, but I would like a confirmation and suitability before proceeding
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From pertusus@free.fr 2007-12-30 20:27 EST ------- (In reply to comment #10)
- Copyright (c) 1986 by University of Toronto.
- Written by Henry Spencer. Not derived from licensed software.
- Permission is granted to anyone to use this software for any
- purpose on any computer system, and to redistribute it freely,
- subject to the following restrictions:
- The author is not responsible for the consequences of use of
this software, no matter how awful, even if they arisefrom defects in it.
- The origin of this software must not be misrepresented, either
by explicit claim or by omission.
- Altered versions must be plainly marked as such, and must not
be misrepresented as being the original software.This looks like a variant of MIT to me, but I would like a confirmation and suitability before proceeding
Indeed it looks like MIT/BSD, but is not exactly any of these license. It seems to be acceptable, even though there is an issue. Indeed, the right to modify isn't explicitly granted. However the point 3. implies that modifications are allowed, as long as they are marked as such. So I'd say that it is acceptable, but upstream/author should be contacted.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-12-31 00:52 EST ------- (In reply to comment #10)
- Copyright (c) 1986 by University of Toronto.
- Written by Henry Spencer. Not derived from licensed software.
- Permission is granted to anyone to use this software for any
- purpose on any computer system, and to redistribute it freely,
- subject to the following restrictions:
- The author is not responsible for the consequences of use of
this software, no matter how awful, even if they arisefrom defects in it.
- The origin of this software must not be misrepresented, either
by explicit claim or by omission.
- Altered versions must be plainly marked as such, and must not
be misrepresented as being the original software.
IMO 1 is not a problem and 2 and 3 are the same as zlib http://fedoraproject.org/wiki/Licensing (GPL compatible) http://www.gzip.org/zlib/zlib_license.html So IMO this can be said as zlib.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From pertusus@free.fr 2007-12-31 02:59 EST ------- It is not like the zlib, since the zlib license explictly allows for alteration:
Permission is granted to anyone to use this software for any purpose, including commercial applications, and to alter it and redistribute it freely, subject to the following restrictions:
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From wolfy@nobugconsulting.ro 2007-12-31 03:33 EST ------- Pat, Mamoru, thank you for your advices. As Pat has said in comment #11, the 3rd sentence in the included license implies that alteration is permitted so I assume there is no problem in treating the license as being zlib. I am tempted to ask for the package to be released as GPLv2+ and have the packager ask upstream for clarification + suggest him to use the zlib license which is very close to what he has used. Would this approach be satisfactory in order to include the package in Fedora ?
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From pertusus@free.fr 2007-12-31 04:32 EST ------- The files under the pseudo zlib license needs not to be fully relicensed, since the license is compatible with the GPL (and LGPL). If the binary is consituted by files under the pseudo zlib license and the GPL it is under the GPL.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From wolfy@nobugconsulting.ro 2008-01-07 16:00 EST ------- To tell the truth, I was not thinking about relicensing the files (because they are compatible with GPL) but of the tag to be used in the spec. I am not sure which one is correct: "GPLv2+ and zlib" or simply "GPLv2+" ?
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From pertusus@free.fr 2008-01-07 17:05 EST ------- Simply GPLv2+ if the files are linked together. See:
http://fedoraproject.org/wiki/Licensing/FAQ
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
wolfy@nobugconsulting.ro changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
------- Additional Comments From wolfy@nobugconsulting.ro 2008-01-07 17:21 EST ------- Package Review for boxes-1.1-3 ==============
Key: - = N/A x = Check ! = Problem ? = Not evaluated
=== REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on:x86_64 [x] Rpmlint output: none [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!] License field in the package spec file matches the actual license. License type: %tag GPLv2; correct tag: GPLv2+ [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. SHA1SUM of package: 1fd6e875e9b3c84f2a71f6df73d9eddb37d11c93 [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages.
=== SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on:Devel/x86_64 [x] Package should compile and build into binary rpms on all supported architectures. Tested on:devel/x86_64 and i386 [x] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane.
=== Issues === 1. the correct license tag is GPLv2+ instead of GPLv2
================ *** APPROVED *** ================
Please fix the license tag before uploading to CVS.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From wolfy@nobugconsulting.ro 2008-01-07 17:23 EST ------- Thanks Thomas for pre-review and Patrice and Mamoru for license guidance.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
------- Additional Comments From jhrozek@redhat.com 2008-01-09 03:06 EST ------- Thanks for all the work done on this review!
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
jhrozek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
------- Additional Comments From jhrozek@redhat.com 2008-01-09 03:07 EST ------- New Package CVS Request ======================= Package Name: boxes Short Description: Draw any kind of box around some given text Owners: jhrozek Branches: F-7 F-8 InitialCC: none Cvsextras Commits: yes
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
------- Additional Comments From kevin@tummy.com 2008-01-09 13:13 EST ------- cvs done.
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: boxes - Draw any kind of box around some given text
https://bugzilla.redhat.com/show_bug.cgi?id=292121
jhrozek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From jhrozek@redhat.com 2008-01-11 03:57 EST ------- Built for rawhide - http://koji.fedoraproject.org/koji/buildinfo?buildID=30902
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=292121
Jakub Hrozek jhrozek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs+ |fedora-cvs?
--- Comment #24 from Jakub Hrozek jhrozek@redhat.com 2010-01-04 08:05:55 EDT --- Package Change Request ====================== Package Name: boxes New Branches: EL-4 EL-5 Owners: jhrozek
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=292121
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #25 from Kevin Fenzi kevin@tummy.com 2010-01-04 15:26:06 EDT --- cvs done.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla