Bug 180319 - Review Request: svnmailer - Tool to post subversion repository commit information
Summary: Review Request: svnmailer - Tool to post subversion repository commit informa...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: John Mahowald
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-02-07 08:58 UTC by Michael Fleming
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-05-08 01:42:09 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Michael Fleming 2006-02-07 08:58:52 UTC
Spec Name or Url: http://www.enlartenment.com/extras/svnmailer.spec
SRPM Name or Url: http://www.enlartenment.com/extras/svnmailer-1.0.6-1.src.rpm
Description: 

Svnmailer is a tool to post subversion repository commit information by mail,
news or XML (to a CIA tracker).

Comment 1 Roozbeh Pournader 2006-02-07 11:36:56 UTC
Random first comments:

* Delete the python_sitearch definition, as it's not used.
* Since this is an original submission, please use a template created by
fedora-newrpmspec and follow that template's style
* Use "BuildArch: noarch" (since this has no arch-dependent file)


Comment 2 Michael Fleming 2006-02-07 11:52:26 UTC
New source RPM up at
http://www.enlartenment.com/extras/svnmailer-1.0.6-2.src.rpm (Old spec replaced
with newer shinier model, same place as before)

- BuildArch: noarch added
- Superflous sitearch definition removed
- The inital spec /was/ based off an older FE python template :-P - I've tried
to undo whatever damage my local revisions have done (I've packaged it locally
before..)

Comment 3 Roozbeh Pournader 2006-02-07 12:39:02 UTC
* I would still prefer a more strict following of the spec template. Use the new
template and fill it with the older information: it would really make it more
readable.
* Change the license to "Apache Software License" so rpmlint likes it.
* Does it really need subversion as a Buildreq?

Suggestions (non-binding!):
* You may consider separating the documentation into a subpackage. It's large
and it probably won't be useful to most of the users.
* You may also consider running the test suite provided upstream automatically.


Comment 4 Michael Fleming 2006-02-08 09:15:02 UTC
(In reply to comment #3)

New SRPM: http://www.enlartenment.com/extras/svnmailer-1.0.6-2.src.rpm
New SPEC: http://www.enlartenment.com/extras/svnmailer.spec

> * I would still prefer a more strict following of the spec template. Use the new
> template and fill it with the older information: it would really make it more
> readable.

Done, hope it's more sensible and legible.

> * Change the license to "Apache Software License" so rpmlint likes it.

Done. How on earth did I miss that before :-P

> * Does it really need subversion as a Buildreq?

Interestingly, yes - as it likes the subversion python bindings there during
builds (and FC4 has them in the base package...).

I've built it before without it and the results are quite erm, *interesting* :-). 

> Suggestions (non-binding!):
> * You may consider separating the documentation into a subpackage. It's large
> and it probably won't be useful to most of the users.

Done (svnmailer-doc - suggestions on a better subpackage convention welcomed)

> * You may also consider running the test suite provided upstream automatically.
> 

Not a bad idea for folks who want to extend or otherwise hack on it, but I'd
like to nail the basic package first. TODO.


Comment 5 Michael Fleming 2006-02-28 08:42:56 UTC
Spec Name or Url: http://www.enlartenment.com/extras/svnmailer.spec
SRPM Name or Url: http://www.enlartenment.com/extras/svnmailer-1.0.7-1.src.rpm

Upgrade to new upstream version (may as well keep it current eh? :-))

Comment 6 Michael Fleming 2006-04-04 08:45:56 UTC
Ping - anyone interested in this still?

I've rebuilt my copy for FC5, seems to work OK.

Comment 7 John Mahowald 2006-05-05 01:04:07 UTC
The documentation may actually be of some use, as it's needed to generate a
config file. But it does make sense to split it off. You may want to remove an
annoying extra doc folder by doing in the %files doc section something like

%doc docs/*

But that's optional. Also a good idea would be a version bump to 1.0.8.

For now:
- rpmlint checks return clean
- package meets naming guidelines
- package meets packaging guidelines
- license (Apache Software License) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on FC5 (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- nothing in %doc affects runtime
- no need for .desktop file

APPROVED

Comment 8 Michael Fleming 2006-05-08 01:42:09 UTC
Imported and a build request on the devel branch queued


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