[Bug 431277] Review Request: ocfs2-tools - programs for managing Ocfs2 file systems

[bugzilla at redhat.com]

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: ocfs2-tools - programs for managing Ocfs2 file systems


https://bugzilla.redhat.com/show_bug.cgi?id=431277





------- Additional Comments From jwilson at redhat.com  2008-02-22 13:42 EST -------
Excellent, looking much better! I have two tiny little changes left for the spec:

1) for the sake of consistency, use either '%{name} = %{version}-%{release}' or
'ocfs2-tools = %{version}-%{release}'  in both ocfs2console and
ocfs2-tools-devel. Right now, one has one form, one has the other. (Actually my
fault, since my earlier patch changed one and not the other)

2) pull the bits about grepping for CFLAGS out of the spec. I assume they were
just there to verify they were getting passed through correctly.

Now for the full review checklist:

Fedora Package Review: ocfs2-tools
----------------------------------

MUST Items:

* rpmlint output acceptable (post full output w/waiver notes where needed):

ocfs2-tools.x86_64: W: service-default-enabled /etc/rc.d/init.d/ocfs2

This one is fine, per earlier discussion in the bug.

ocfs2-tools.x86_64: W: service-default-enabled /etc/rc.d/init.d/o2cb
ocfs2-tools.x86_64: E: subsys-not-used /etc/rc.d/init.d/o2cb

This script is a bit terrifying... Can you elaborate a bit on what this one does
and why it also needs to be on by default?

ocfs2-tools-devel.x86_64: W: no-documentation

Not a problem.


* Meets Package Naming Guidelines

* spec file name matches %{name}, in the format %{name}.spec

* The package meets the Packaging Guidelines

* open-source compatible license and meets fedora legal reqs (GPLv2)

* License field in spec matches actual license

* Includes text of license(s) in its own file, file in %doc

* spec file legible and in American English

* sources used match the upstream source, as provided in spec URL. Verify with
md5sum (if no upstream URL, source creation method must be documented and can be
verified using diff).

* produces binary rpms on at least one arch (tested f8/x86_64)

* No ExcludeArch

* BuildRequires are sane

* no locales

* no shared libs

* not relocatable

* package owns all directories it creates

* no duplicates in %files

* Permissions on %files sane

* %clean includes rm -rf %{buildroot}/$RPM_BUILD_ROOT

* macros used consistently

* package contains code

* No need for docs sub-package

* files in %doc aren't required for package to work

* Header files in -devel package

* Static libs present, explained, and packaged in accordance with the guidelines

* package Reqs: pkgconfig if pkgconfig(.pc) files present

* no versioned libs

* -devel packages requires base package NVR

* no libtool archives

* GUI app (ocfs2console), includes a %{name}.desktop file, installed with
desktop-file-install in the %install section 

* doesn't own files or folders other package own

* %install starts with rm -rf %{buildroot}/$RPM_BUILD_ROOT

* filenames in packages are valid UTF-8


SHOULD Items (not absolutely mandatory, but highly encouraged)

* package should build in mock: built in f8/x86_64 mock chroot

* package should build on all supported architectures: only tested x86_64 myself

* package should function as expected: untested by me, but I assume it does

* scriptlets are sane

* subpackages other than -devel require the base package using a fully versioned
dependency

* pkgconfig files in -devel pkg



So basically, just the two little things on the spec side, and a bit more
explanation of the o2cb initscript, and I'm prepared to approve the package and
do the whole sponsor thing. :)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.