Bug 426349 - Review Request: mono-addins
Summary: Review Request: mono-addins
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Nielsen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 428581
TreeView+ depends on / blocked
 
Reported: 2007-12-20 11:47 UTC by Paul F. Johnson
Modified: 2008-02-23 16:45 UTC (History)
5 users (show)

Fixed In Version: mono-addins-0.3-4.fc9.x86_64
Clone Of:
Environment:
Last Closed: 2008-02-23 16:45:44 UTC
Type: ---
Embargoed:
gnomeuser: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Paul F. Johnson 2007-12-20 11:47:05 UTC
Spec URL: http://pfj.fedorapeople.org/mono-addins.spec
SRPM URL: http://pfj.fedorapeople.org/mono-addins-1.2.6-1.fc9.src.rpm
Description: mono-addins is required for monodevelop and is currently expected to take over infrastructure aspects of mono (such as for monodoc etc)

Comment 1 Timon 2007-12-21 07:19:04 UTC
wrong link to src package, should be
http://pfj.fedorapeople.org/mono-addins-0.3-1.fc9.src.rpm
in spec

URL: http://www.mono-project.com/Mono.Addins
Source0: http://www.go-mono.com/sources/mono-addins/%{name}-%{version}.tar.bz2

rpmlint:
mono-addins.i386: W: no-documentation
mono-addins.i386: W: invalid-license GPL
mono-addins.i386: E: no-binary
mono-addins.i386: E: only-non-binary-in-usr-lib

There is no information about license of Mono.Addins, should ask mainstream.

rpmlint mono-addins-devel-0.3-1.fc8.i386.rpm 
mono-addins-devel.i386: W: no-documentation
mono-addins-devel.i386: W: summary-not-capitalized development files for mono-addins
mono-addins-devel.i386: W: non-standard-group Development/Applications
mono-addins-devel.i386: W: invalid-license GPL


Comment 2 David Nielsen 2007-12-28 18:46:45 UTC
Timon are you doing the review? If not I would be happy to take this, if you are please assign the package 
to yourself.

Paul are you working on packaging Mono.Zeroconf, I know this will be in Mono 1.2.7 but till then it is needed 
to build Banshee SVN (and thus likely the next stable Banshee - you see my obvious interest in tricking you 
into packaging it here, I'll be happy to review it)

Comment 3 Paul F. Johnson 2007-12-29 23:01:53 UTC
Do you have a URL for Mono.Zeroconf?

Comment 4 David Nielsen 2007-12-30 00:43:25 UTC
http://mono-project.com/Mono.Zeroconf

Comment 5 Timon 2007-12-30 06:07:04 UTC
I'm too young for doing a review. :) sorry. and I don't know how to do a review :(

Comment 6 David Nielsen 2007-12-30 10:48:24 UTC
funny since you were off to a good start, regardless I'll claim this for great
justice. Paul in the future just cc me to review requests, I'd love to see Mono
do better in Fedora so I'm all for streamlining the progress of getting your
packages reviewed.

Comment 7 Paul F. Johnson 2008-01-03 23:23:31 UTC
#6 thanks!

I've uploaded a newer version tonight which addresses much of #1's concerns

Timon : thanks for the initial review. Mono packages can be an utter swine to
get right as they are somewhat insane! The errors thrown up aren't actually
errors in this case, which tends to throw some.

As David said, keep it up!

Comment 8 Paul F. Johnson 2008-01-06 22:37:46 UTC
David - you up for reviewing this?

Comment 9 David Nielsen 2008-01-10 03:18:03 UTC
I apologize, lots of shit going on so I kinda dropped the ball on this..

BAD:
mono-addins.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 7)
mono-addins-devel.x86_64: W: summary-not-capitalized development files for
mono-addins
mono-addins-devel.x86_64: W: non-standard-group Development/Applications
mono-addins-debuginfo.x86_64: E: empty-debuginfo-package
Fails to build in mock (fedora-8-i386) due to missing dependency fulfilling
autoreconf

GOOD:
Owns all it's files
SPEC is in clean US English

MERGH?:
Are the --prefix/libdir configure arguments really needed?
It's good form to name patches after it's function instead of just the package
name (so in this case mono-addins-libdir.patch would be good I suppose).

Comment 10 Paul F. Johnson 2008-01-14 11:59:46 UTC
Spec URL: http://pfj.fedorapeople.org/mono-addins.spec
SRPM URL: http://pfj.fedorapeople.org/mono-addins-1.2.6-3.fc9.src.rpm

Fixed all but MERGH?:

I've found that with some mono packages, exporting the --prefix/libdir stuff
solves more problems than fixing the makefiles, so I do it now just as a belt
and braces sort of thing.

Comment 11 David Nielsen 2008-01-14 15:12:19 UTC
The thing is.. we already export those as part of the rpm build system, that
shouldn't be needed. If it makes a difference then I think it's a bug with our
build magic not your spec and should be handled as such.

BAD:
mono-addins.src: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 3)

However it now builds in mock and I have no further complains, fix that tabs
spaces thing before checking it though.

APPROVED

Comment 12 Paul F. Johnson 2008-01-14 18:33:38 UTC
I'll fix the W before import


Comment 13 David Nielsen 2008-01-14 18:39:32 UTC
oh and excludearch ppc64 on account of #241850 - forgot that one before because
I don't have ppc hardware but if you don't do that koji will give you the seal
of epic fail.

Comment 14 Paul F. Johnson 2008-01-14 18:45:07 UTC
New Package CVS Request
=======================
Package Name: mono-addins
Short Description: mono-addins is required for monodevelop and is currently
expected to take over infrastructure aspects of mono
Owners: pfj
Branches: f8, f7
InitialCC: pfj
Cvsextras Commits: yes


Comment 15 Kevin Fenzi 2008-01-14 21:45:02 UTC
Is that short description right? Typically you would want to use the Summary
from the package spec, in this case "Addins for mono"?

Comment 16 Paul F. Johnson 2008-01-16 10:45:58 UTC
Yes. I'll amend the one in the spec file before uploading.

Comment 17 Paul F. Johnson 2008-01-16 10:47:04 UTC
Sorry, misread. 

New Package CVS Request
=======================
Package Name: mono-addins
Short Description: Addins for mono
Owners: pfj
Branches: f8, f7
InitialCC: pfj
Cvsextras Commits: yes


Comment 18 Kevin Fenzi 2008-01-16 17:34:53 UTC
cvs done. Sorry for the delay/hassle. 

Comment 19 David Nielsen 2008-02-23 16:45:44 UTC
Now in the repos, CLOSING


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