Bug 485007 - (rhnpush) Review Request: rhnpush - Package uploader for the RHN Satellite/Spacewalk Server
Review Request: rhnpush - Package uploader for the RHN Satellite/Spacewalk Se...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Marcela Mašláňová
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks: F-Spacewalk
  Show dependency treegraph
 
Reported: 2009-02-10 23:05 EST by Michael Stahnke
Modified: 2009-02-27 05:26 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-02-27 05:26:08 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mmaslano: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Michael Stahnke 2009-02-10 23:05:52 EST
Spec URL: http://stahnma.fedorapeople.org/reviews/rhnpush.spec
SRPM URL: http://stahnma.fedorapeople.org/reviews/rhnpush-0.3.1-2.fc10.src.rpm
Description: rhnpush uploads package headers to the Red Hat Network servers into
specified channels and allows for several other channel management
operations relevant to controlling what packages are available per
channel.
Comment 1 Marcela Mašláňová 2009-02-13 08:46:35 EST
FAIL source files match upstream
The source should be written as whole path.
OK package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
OK dist tag is present.
OK build root is correct.
OK license field (same as Perl) matches the actual license.
OK license is open source-compatible. License text not included upstream.
OK latest version is being packaged.
OK BuildRequires are proper.
OK %clean is present.
OK package builds in mock (Rawhide/x86_64).
http://koji.fedoraproject.org/koji/taskinfo?taskID=1124650
OK debuginfo package isn't need.
OK rpmlint is silent.
OK final provides and requires look sane.
OK no shared libraries are added to the regular linker search paths.
OK owns the directories it creates.
OK no duplicates in %files.
OK file permissions are appropriate.
OK no scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK no headers.
OK no pkgconfig files.
OK no libtool .la droppings.

Shouldn't be test executed as a part of build process?
(btw you have ugly man page of rhnpush the synopsis part.)
Comment 2 Michael Stahnke 2009-02-13 21:31:37 EST

Spec URL: http://stahnma.fedorapeople.org/reviews/rhnpush.spec
SRPM URL: http://stahnma.fedorapeople.org/reviews/rhnpush-0.3.1-3.fc10.src.rpm

* I have updated the package slightly.  I added a source description, as per 
     http://fedoraproject.org/wiki/Packaging/SourceURL .
* I tried to play with %check and tests, but I couldn't them to work.  It seems like I am missing a harness setup of some type.  I will ping the sw-dev list and see if I can find out anything.  Either way, that shouldn't be a blocker for package review.

* Man page:  It's a little different than many, but certainly not wrong.  See (man vim), it looks similar.  I think it's because the man page is generated using doc2book on the SGML file.  I'd rather just write it in groff, but I am not upstream.
Comment 3 Michael Stahnke 2009-02-13 21:48:42 EST
Spec URL: http://stahnma.fedorapeople.org/reviews/rhnpush.spec
SRPM URL: http://stahnma.fedorapeople.org/reviews/rhnpush-0.3.1-4.fc10.src.rpm


* Found a better Source URL.
Comment 4 Marcela Mašláňová 2009-02-17 09:28:42 EST
But if I do it then I have 7f847b5b6066ced228e7b7868524cdcf  rhnpush-0.4.2.tar.gz. So we have two problems instead one. You should package the latest version and here are different sources :)

The other issues were only comments, nothing serious.
Comment 5 Michael Stahnke 2009-02-17 10:48:35 EST
If I am reading this correctly, I should package the released Tarball and not the generated tarball from Git.  Is that correct?
Comment 6 Marcela Mašláňová 2009-02-18 04:00:09 EST
You can leave source code as you have it, but if Fedora is upstream you should upload tarball on proper place on https://fedorahosted.org/releases/

Also there is condition that you should package the latest release.
Comment 7 Miroslav Suchý 2009-02-20 08:03:52 EST
Michael if you do not mind (and I know you will not :) ) I will take ownership of this package. I really want to thanx you for all the job you are doing.
I applied your changes to our git repo. I packed latest version. And make sure the tar.gz landed in fedorahosted.org/releases/

*Test - the test are long time abandoned and definitelly do not work. We keep it there if anybody want to fix it as it is better then just deleting it forever.

UPDATED 
SPEC: http://miroslav.suchy.cz/fedora/rhnpush/rhnpush.spec
SRPM: http://miroslav.suchy.cz/fedora/rhnpush/rhnpush-0.4.3-1.src.rpm
Comment 8 Marcela Mašláňová 2009-02-20 08:22:59 EST
OK source files match upstream 3b53e8a569b7e486634482216410669d
OK package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
OK dist tag is present.
OK build root is correct.
OK license field (GPLv2) matches the actual license.
OK license is open source-compatible. License text not included upstream.
OK latest version is being packaged.
OK BuildRequires are proper.
OK %clean is present.
OK package builds in mock (Rawhide/x86_64).
http://koji.fedoraproject.org/koji/taskinfo?taskID=1142201
OK debuginfo package isn't need.
OK rpmlint is silent.
OK final provides and requires look sane.
OK no shared libraries are added to the regular linker search paths.
OK owns the directories it creates.
OK no duplicates in %files.
OK file permissions are appropriate.
OK no scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK no headers.
OK no pkgconfig files.
OK no libtool .la droppings.

What about BR? Is there reason for using %{_bindir}/msgfmt instead of gettext and %{_bindir}/docbook2man instead of docbook-utils?
Comment 9 Miroslav Suchý 2009-02-20 08:46:57 EST
Hmm, actually there is no reason. And yes, package dependence should be prefered rather then file dependence.

UPDATED 
SPEC: http://miroslav.suchy.cz/fedora/rhnpush/rhnpush.spec
SRPM: http://miroslav.suchy.cz/fedora/rhnpush/rhnpush-0.4.4-1.src.rpm
Comment 10 Marcela Mašláňová 2009-02-20 08:56:50 EST
ACCEPTED
Comment 11 Miroslav Suchý 2009-02-20 09:14:45 EST
New Package CVS Request
=======================
Package Name: rhnpush
Short Description: Package uploader for the RHN Satellite/Spacewalk Server
Owners: msuchy
Branches: F-10, EL-4, EL-5
InitialCC: stahnma
Comment 12 Kevin Fenzi 2009-02-26 19:20:42 EST
cvs done.

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