Bug 786213

I'll try and find time to review this. Anyone else welcome to review in the mean time. ;)

Before I start in on the review here, could you update to the latest upstream?

agilo-0.9.6.2 seems to be the current version.

All the dependencies are now present in Fedora, so the package can be built in Koji.

Spec URL: http://kklic.fedorapeople.org/trac-agilo-plugin.spec
SRPM URL: http://kklic.fedorapeople.org/trac-agilo-plugin-0.9.7-1.fc16.src.rpm

* Mon Mar 19 2012 Karel Klíč <kklic> - 0.9.7-1
- Update to the newest upstream release

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License (ASL 2.0)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
fe527f0b0a2391394e4c7139b8718dae  agilo_source.tar.gz
fe527f0b0a2391394e4c7139b8718dae  agilo_source.tar.gz.orig

OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
See below - Package has a correct %clean section. 
See below - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
See below - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
See below - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should function as described. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. You might ask upstream to include a copy of the license. 
Not a blocker though. 

2. You don't need a builtroot or clean section for Fedora anymore, but 
you do for EPEL6, so you might add those back for that. 

3. You need a
rm -rf %{buildroot}
at the top of the install section. 

4. rpmlint says: 

Can be ignored, but man pages would be nice: 

trac-agilo-plugin.noarch: W: no-manual-page-for-binary agilo_sqlite2pg
trac-agilo-plugin.noarch: W: no-manual-page-for-binary create_agilo_project
trac-agilo-plugin.noarch: W: no-manual-page-for-binary agilo_svn_hook_commit

You should change all the commented macros to %%

trac-agilo-plugin.src:30: W: macro-in-comment %check
trac-agilo-plugin.src:31: W: macro-in-comment %{python_sitelib}
trac-agilo-plugin.src:32: W: macro-in-comment %{python_sitelib}
trac-agilo-plugin.src:33: W: macro-in-comment %{_defaultdocdir}
trac-agilo-plugin.src:33: W: macro-in-comment %{VERSION}
trac-agilo-plugin.src:34: W: macro-in-comment %{buildroot}
trac-agilo-plugin.src:34: W: macro-in-comment %{python_sitelib}
trac-agilo-plugin.src:34: W: macro-in-comment %{__python}

Can be ignored: 

trac-agilo-plugin.src: W: invalid-url Source0: agilo_source.tar.gz

5. Is there a specific version of trac thats required?

Kevin, thank you for the review!

Here is an updated version:

Spec URL: http://kklic.fedorapeople.org/trac-agilo-plugin.spec
SRPM URL: http://kklic.fedorapeople.org/trac-agilo-plugin-0.9.7-2.fc16.src.rpm

* Wed Mar 28 2012 Karel Klíč <kklic> - 0.9.7-2
- Commented macros changed to %%


(In reply to comment #4)
> Issues: 
> 
> 1. You might ask upstream to include a copy of the license. 
> Not a blocker though. 
> 
> 2. You don't need a builtroot or clean section for Fedora anymore, but 
> you do for EPEL6, so you might add those back for that. 

It seems that the BuildRoot tag and the clean section are not required in EPEL6:

http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Distribution_specific_guidelines

> 
> 3. You need a
> rm -rf %{buildroot}
> at the top of the install section. 

This is required only for EPEL5 and older:

http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Prepping_BuildRoot_For_.25install

> 
> 4. rpmlint says: 
> 
> Can be ignored, but man pages would be nice: 
> 
> trac-agilo-plugin.noarch: W: no-manual-page-for-binary agilo_sqlite2pg
> trac-agilo-plugin.noarch: W: no-manual-page-for-binary create_agilo_project
> trac-agilo-plugin.noarch: W: no-manual-page-for-binary agilo_svn_hook_commit
> 
> You should change all the commented macros to %%
> 
> trac-agilo-plugin.src:30: W: macro-in-comment %check
> trac-agilo-plugin.src:31: W: macro-in-comment %{python_sitelib}
> trac-agilo-plugin.src:32: W: macro-in-comment %{python_sitelib}
> trac-agilo-plugin.src:33: W: macro-in-comment %{_defaultdocdir}
> trac-agilo-plugin.src:33: W: macro-in-comment %{VERSION}
> trac-agilo-plugin.src:34: W: macro-in-comment %{buildroot}
> trac-agilo-plugin.src:34: W: macro-in-comment %{python_sitelib}
> trac-agilo-plugin.src:34: W: macro-in-comment %{__python}

Done.

> 
> Can be ignored: 
> 
> trac-agilo-plugin.src: W: invalid-url Source0: agilo_source.tar.gz
> 
> 5. Is there a specific version of trac thats required?

Yes, the upstream package requires trac >= 0.11.  This condition is satisfied in all Fedora branches and EPEL6 branch.

Do we have a policy or convention to add the versioned require to the spec file?  I propose adding the version when the non-versioned require one will cause problems somewhere.

If all supported branches have the requirement you don't really need to specify. 
I guess it's a matter of taste. The only place it could be bad is if someone took your src.rpm and tried to build it for another branch with a different version, they could be confused when it fails... but not a big deal. 

Great. That answers all my issues/questions. 

I see no further blockers here, so this package is APPROVED. 

Happy to help co-maintain if you like.

Thanks, your co-maintainership is welcome.

New Package SCM Request
=======================
Package Name: trac-agilo-plugin
Short Description: A plugin for supporting the Scrum process in Trac
Owners: kklic kevin
Branches: f16 f17 el6
InitialCC:

Git done (by process-git-requests).

trac-agilo-plugin-0.9.7-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/trac-agilo-plugin-0.9.7-2.fc17

trac-agilo-plugin-0.9.7-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/trac-agilo-plugin-0.9.7-2.fc16

Package trac-agilo-plugin-0.9.7-2.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing trac-agilo-plugin-0.9.7-2.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-4942/trac-agilo-plugin-0.9.7-2.fc17
then log in and leave karma (feedback).

trac-agilo-plugin-0.9.7-2.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

trac-agilo-plugin-0.9.7-2.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.

trac-agilo-plugin-0.9.7-2.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

trac-agilo-plugin-0.9.7-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/trac-agilo-plugin-0.9.7-2.el6

trac-agilo-plugin-0.9.7-2.el6 has been pushed to the Fedora EPEL 6 stable repository.