This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 207802 - Review Request: libpaper - Library and tools for handling papersize
Review Request: libpaper - Library and tools for handling papersize
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dan Horák
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 207761
  Show dependency treegraph
 
Reported: 2006-09-23 09:53 EDT by Tom "spot" Callaway
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-23 12:38:33 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Tom "spot" Callaway 2006-09-23 09:53:04 EDT
Spec URL: http://www.auroralinux.org/people/spot/review/libpaper.spec
SRPM URL: http://www.auroralinux.org/people/spot/review/libpaper-1.1.20-1.fc6.src.rpm
Description: 
The paper library and accompanying files are intended to provide a
simple way for applications to take actions based on a system- or
user-specified paper size. This release is quite minimal, its purpose
being to provide really basic functions (obtaining the system paper name
and getting the height and width of a given kind of paper) that
applications can immediately integrate.
Comment 1 Dan Horák 2006-09-23 10:49:01 EDT
rpmlint from Rawhide gives
	W: libpaper mixed-use-of-spaces-and-tabs (on the src.rpm)

Could the static library be excluded?

When running paperconf, it could not find /etc/papersize.
Comment 2 Tom "spot" Callaway 2006-09-23 11:03:52 EDT
/etc/papersize is (re)generated with /usr/sbin/paperconfig, but the package
should own it. I set letter as the default.

New SRPM:
http://www.auroralinux.org/people/spot/review/libpaper-1.1.20-2.fc6.src.rpm
New SPEC:
http://www.auroralinux.org/people/spot/review/libpaper.spec



Comment 3 Dan Horák 2006-09-23 12:09:28 EDT
Review:
- no rpmlint output on any package
- package name OK
- spec file name OK, is in English and is legible
- package meets the Packaging Guidelines
- license OK (GPL) and is included
- source matches upstream
	7075f580606a84e63b7d6d9fa3124c31  libpaper_1.1.20.tar.gz
	7075f580606a84e63b7d6d9fa3124c31  libpaper_1.1.20.tar.gz.1
- compiles and builds at least on i386 (devel)
- BuildRequires are correct
- contains no localized files
- has shared lib with appropriate ldconfig calls
- does not create any directory
- no duplicates files, permissions are set properly, uses %defattr
- has %clean section
- consistent use of macros
- contains code
- no large docs, %doc is not required during runtime
- devel subpackage required and present, contains header
- no .la libtool archives
- not a GUI application
- it works

APPROVED

The static library can be taken out also with %configure --disable-static.
Comment 4 Tom "spot" Callaway 2006-09-23 12:38:33 EDT
Thanks for the fast review, built in FC-5 and devel.
Comment 5 Patrice Dumas 2006-09-23 13:20:40 EDT
I have spotted some more or less minor issues:

* the man pages for functions should be in the devel 
  package

%{_mandir}/man3/*

* removing unneded files
rm -rf $RPM_BUILD_ROOT%{_libdir}/*.la
rm -rf $RPM_BUILD_ROOT%{_libdir}/*.a
should be without -r and even maybe without f

* I think that there is no need to set letter as a default, it 
  is allready the default according to the man paperconf. In my 
  opinion, what should be better is an empty file or a file with a 
  comment along 
echo '# Simply write the paper name. See papersize(5) for possible values' > 
$RPM_BUILD_ROOT%{_sysconfdir}/papersize

* in man paperconfig there is a reference to /etc/libpaper.d

* The file NEWS in debian/ should certainly be in %doc
Comment 6 Dan Horák 2006-09-23 13:56:08 EDT
Yes, I really missed the man3 directory. The rest is minor, but reasonable.

Also in debian/po subdirectory there are hidden localized files. It would be
nice to have them installed.

Comment 7 Tom "spot" Callaway 2006-09-23 21:09:37 EDT
Thanks for the feedback. -3 should resolve all the above issues.

Note You need to log in before you can comment on or make changes to this bug.