Bug 786213 - Review Request: trac-agilo-plugin - A plugin for supporting the Scrum process in Trac
Summary: Review Request: trac-agilo-plugin - A plugin for supporting the Scrum process...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 786093
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-31 18:07 UTC by Karel Klíč
Modified: 2013-03-03 23:03 UTC (History)
5 users (show)

Fixed In Version: trac-agilo-plugin-0.9.7-2.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-11 04:00:45 UTC
Type: ---
Embargoed:
kevin: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Karel Klíč 2012-01-31 18:07:13 UTC
Spec URL: http://kklic.fedorapeople.org/trac-agilo-plugin.spec
SRPM URL: http://kklic.fedorapeople.org/trac-agilo-plugin-0.9.5-1.fc16.src.rpm

Description:
Agilo for Trac is a simple and straightforward tool to support the
Scrum process.


rpmlint trac-agilo-plugin-0.9.5-1.fc16.src.rpm 
trac-agilo-plugin.src:27: W: macro-in-comment %check
trac-agilo-plugin.src:28: W: macro-in-comment %{python_sitelib}
trac-agilo-plugin.src:29: W: macro-in-comment %{python_sitelib}
trac-agilo-plugin.src:30: W: macro-in-comment %{_defaultdocdir}
trac-agilo-plugin.src:30: W: macro-in-comment %{VERSION}
trac-agilo-plugin.src:31: W: macro-in-comment %{buildroot}
trac-agilo-plugin.src:31: W: macro-in-comment %{python_sitelib}
trac-agilo-plugin.src:31: W: macro-in-comment %{__python}
trac-agilo-plugin.src: W: invalid-url Source0: agilo_source.tar.gz
1 packages and 0 specfiles checked; 0 errors, 9 warnings.

rpmlint trac-agilo-plugin-0.9.5-1.fc16.noarch.rpm 
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
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

Comment 1 Kevin Fenzi 2012-02-06 20:07:04 UTC
I'll try and find time to review this. Anyone else welcome to review in the mean time. ;)

Comment 2 Kevin Fenzi 2012-02-17 21:23:55 UTC
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.

Comment 3 Karel Klíč 2012-03-19 06:18:43 UTC
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

Comment 4 Kevin Fenzi 2012-03-21 16:22:35 UTC
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?

Comment 5 Karel Klíč 2012-03-28 15:17:18 UTC
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.

Comment 6 Kevin Fenzi 2012-03-28 17:54:32 UTC
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.

Comment 7 Karel Klíč 2012-03-30 10:45:07 UTC
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:

Comment 8 Gwyn Ciesla 2012-03-30 12:49:51 UTC
Git done (by process-git-requests).

Comment 9 Fedora Update System 2012-03-30 14:25:28 UTC
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

Comment 10 Fedora Update System 2012-03-30 14:25:39 UTC
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

Comment 11 Fedora Update System 2012-03-30 18:01:33 UTC
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).

Comment 12 Fedora Update System 2012-04-11 04:00:45 UTC
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.

Comment 13 Fedora Update System 2012-04-11 16:52:11 UTC
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.

Comment 14 Fedora Update System 2012-04-12 02:28:42 UTC
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.

Comment 15 Fedora Update System 2012-04-20 09:35:52 UTC
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

Comment 16 Fedora Update System 2012-05-17 20:31:33 UTC
trac-agilo-plugin-0.9.7-2.el6 has been pushed to the Fedora EPEL 6 stable repository.


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