Bug 439117 - Review Request: preupgrade - Preresolves dependencies and prepares a system for an upgrade
Summary: Review Request: preupgrade - Preresolves dependencies and prepares a system f...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-03-27 02:52 UTC by Seth Vidal
Modified: 2008-05-14 19:49 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-05-14 19:49:10 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Seth Vidal 2008-03-27 02:52:26 UTC
Spec URL: http://skvidal.fedorapeople.org/preupgrade/preupgrade.spec
SRPM URL: http://skvidal.fedorapeople.org/preupgrade/preupgrade-0.8-1.src.rpm
Description: Preresolves all dependencies, downloads the packages and makes your system ready for an upgrade via anaconda.

Comment 1 Colin Walters 2008-03-30 03:42:24 UTC
This wasn't building in mock for me; missing BuildRequires on python?

Comment 2 Will Woods 2008-04-03 22:50:45 UTC
Added. Builds in mock for me now.

Spec URL: http://wwoods.fedorapeople.org/review/preupgrade.spec
SRPM URL: http://wwoods.fedorapeople.org/review/preupgrade-0.9-1.src.rpm

Comment 3 Jason Tibbitts 2008-04-05 01:41:38 UTC
Builds fine.  rpmlint says:
  preupgrade.src: W: strange-permission preupgrade.spec 0600
which is a little weird but isn't a blocker.  (If it were 666, that would be a
problem.)

Is there no URL where the source can be downloaded?  Where does the tarball come
from?

BuildRoot should be one of the values from
http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot or at minimum
should include %{release} in addition to what's there.

You can shorten BuildArchitectures: as BuildArch: if you like to save typing. 
(I only mention it because vim highlights it oddly; the 'itectures' is a
different color.)

You can remove the tests that ensure the buildroot isn't '/'; rpmbuild does that
for you.

I was under the impression that Red Hat-developed code was GPLv2 only.  Not
really my business as the License: tag matches the source code, but I figured
I'd ask.

I notice you don't use the dist tag.  Your choice, of course; I assume you know
how to deal with its absence.

? can't check that the sources match upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
X build root is not OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   config(preupgrade) = 0.9-1
   preupgrade = 0.9-1
  =
   /bin/sh
   /usr/bin/python
   config(preupgrade) = 0.9-1
   python >= 2.1
   python(abi) = 2.5
   rpm >= 0:4.1.1
   rpm-python
   usermode
   yum >= 3.2.8
   yum-metadata-parser

* %check is not present; no test suite upstream.
  I installed this on a rawhide box and it ran well enough, although it doesn't 
  really do much in that case.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 4 Milos Jakubicek 2008-04-05 23:50:45 UTC
Should/Can I report also bugs (not concerning packaging) now or should I wait
until the review process is accomplished and file then bugs against this
component when it will be created in bugzilla?

Comment 5 Jason Tibbitts 2008-04-06 18:30:43 UTC
Milos,

This isn't really the best place to report bugs in preupgrade, since this ticket
will quickly become impossible to follow and issues wouldn't be able to be
tracked individually.

My recommendation would be to go to the upstream web site and file tickets in
their track instance.  https://fedorahosted.org/preupgrade/


Comment 6 Seth Vidal 2008-04-07 16:51:27 UTC
(In reply to comment #3)
> Builds fine.  rpmlint says:
>   preupgrade.src: W: strange-permission preupgrade.spec 0600
> which is a little weird but isn't a blocker.  (If it were 666, that would be a
> problem.)

artifact of rpmbuild -ts file.tar.gz :)


 
> Is there no URL where the source can be downloaded?  Where does the tarball come
> from?

not yet, we've not exactly made an official release, yet.

 
> BuildRoot should be one of the values from
> http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot or at minimum
> should include %{release} in addition to what's there.

fixed.

 
> You can shorten BuildArchitectures: as BuildArch: if you like to save typing. 
> (I only mention it because vim highlights it oddly; the 'itectures' is a
> different color.)

fixed.

> 
> You can remove the tests that ensure the buildroot isn't '/'; rpmbuild does that
> for you.

fixed.

> 
> I was under the impression that Red Hat-developed code was GPLv2 only.  Not
> really my business as the License: tag matches the source code, but I figured
> I'd ask.

that's news to me. Where did you see that?

> 
> I notice you don't use the dist tag.  Your choice, of course; I assume you know
> how to deal with its absence.

we'll add it b/c we will need it.

 


Comment 7 Seth Vidal 2008-04-07 19:08:29 UTC
okay so:
1. added the source path and url
2. put up a new src.rpm and spec file at:
 
http://skvidal.fedorapeople.org/preupgrade/

let me know if this passes muster.


Comment 8 Jason Tibbitts 2008-04-08 03:36:09 UTC
Yeah, this looks fine.  The buildroot's good, source URL is good, all of the
other minor issues are fixed, and it still builds.

APPROVED

Comment 9 Seth Vidal 2008-04-08 20:45:08 UTC
New Package CVS Request
=======================
Package Name: preupgrade
Short Description: Preresolves dependencies and prepares a system for an upgrade
Owners: skvidal, wwoods, katzj
Branches: F8, devel
InitialCC: 
Cvsextras Commits: yes


Comment 10 Kevin Fenzi 2008-04-08 20:50:31 UTC
cvs done.

Comment 11 seth vidal 2008-04-16 19:42:53 UTC
New Package CVS Request
=======================
Package Name: preupgrade
Short Description: Preresolves dependencies and prepares a system for an upgrade
Owners: skvidal, wwoods, katzj
Branches: F7
InitialCC: 
Cvsextras Commits: yes


Requesting just the F7 branch addition.


Comment 12 Kevin Fenzi 2008-04-16 21:29:16 UTC
cvs done.


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