[Bug 226211] Merge Review: openhpi

[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.


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

Michal Hlavinka <mhlavink at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |RAWHIDE
               Flag|fedora-review?              |fedora-review+

--- Comment #5 from Michal Hlavinka <mhlavink at redhat.com> 2010-01-13 08:02:39 EST ---
(In reply to comment #4)
> (In reply to comment #3)
> > comments:
> > 
> > 1) rpmlint *.spec *.src.rpm x86_64/*
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libsnmp_bc.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libipmi.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libsimulator.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libwatchdog.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libipmidirect.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/liboa_soap.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libilo2_ribcl.so
> > openhpi.x86_64: W: devel-file-in-non-devel-package
> > /usr/lib64/openhpi/libsysfs2hpi.so
> > 
> > https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages :
> > """.... The following are examples of file types which should be in -devel:
> >     * Header files (e.g. .h files)
> >     * Unversioned shared libraries (e.g. libfoo.so).
> > """
> > 
> > these files should go to -devel package
> 
> these files are plugins loaded with dlopen from the main process, no libraries,
> so they belong to the main package

ok

> 
> > ---------
> > 
> > openhpi.x86_64: E: non-standard-dir-perm /var/lib/openhpi 01777
> > 
> > are these permissions really required? I've done only quick testing/googling
> > (without proper configuration), but didn't find anything about this
> 
> this is what upstream uses, but I can ask them about reasoning

if it's really suggested by upstream, it's ok

> > -----------------
> > 
> > openhpi-libs.x86_64: W: obsolete-not-provided openhpi
> > 
> > why openhpi-libs obsoletes openhpi? for version specified, there were no
> > openhpi-libs provided, but this line would lead to yum replacing openhpi with
> > just openhpi-libs -> openhpid and other files will be missing
> > 
> > what is your rationale for this?
> 
> this is a result from splitting the openhpi package into openhpi and
> openhpi-libs
> (https://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks#Splitting_libraries_into_separate_packages
> - can't find a more official doc now)

ok, fine for me

> > 2) Correct english - see WordUsage.html
> > 
> > %description
> > 
> > hot swap
> > --------
> >     Correct. Two words, lower case. Capitalize when used at the beginning of a
> > sentence only. Do not use ‘hotswap’ or ‘hot-swap’.
> > 
> > plug-in
> > -------
> >     Correct. Do not use "plugin".
> >     A hardware or software module that adds a specific feature or service to a
> > larger system. For example, a number of plug-ins are available for the Netscape
> > Navigator browser that enable it to display different types of audio or video
> > messages. Navigator plug-ins are based on MIME file types.
> > 
> > but these are not blockers ;-)
> 
> hm, I didn't expect a spell-checker in review :-)
> 

it depends how you interpret 'must: The spec file must be written in American
English.' As I've already said, this is not a blocker, but using correct words
is going to make your packages look more professional. No need to rebuild, but
commit itself would be nice :-)

> > 
> > 3) too much wildcards under %files section
> > 
> > If upstream makes some changes to it's tarball and add/remove some files, this
> > is not going to catch anything. It's good practice to list at least all files
> > under %{_bindir} and %{_sbindir}. This will let you know if there is any
> > new/missing one.
> 
> I don't agree. Using wildcards copies upstream intentions what belongs where.
> There are other tools and processes that should check differences between 2
> versions of the package.

well, if openhpi-2.14.1 would have /bin/foo and /bin/bar and openhpi-2.14.2
would have only /bin/foo for example because /bin/bar requires new ./configure
option or build dependency, you won't catch this change with wildcards. Having
(manually) filled %files section with just wildcards is imho pointless. Anyway,
this is not explicitly written in policy, so even if I disagree, this is not a
blocker

> > 4) sources does not match upstream
> > $ curl -s http://downloads.sourceforge.net/openhpi/openhpi-2.14.1.tar.gz |
> > md5sum
> > d41d8cd98f00b204e9800998ecf8427e  -
> > $ cat sources
> > 1533972a05f2ed61f3ae441ecd3df5a9  openhpi-2.14.1.tar.gz
> 
> running spectool -g on the spec file returns the right source archive (with the
> 1533... md5sum)

ok, source file is correct, odd this does not work here, because it was working
earlier for other sources.

to sum it up: no blockers remains, closing

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.