Bug 619437

This is not a full review just a couple of things that stick out in the spec file.

1) The license should be GPLv2+
2) Source: should be Source0: http://including a URL to download original tarball
3) Spec missing %changelog section.
4) Group should be Development/Tools

Is this your first package?

> 1) The license should be GPLv2+
Fixed

> 2) Source: should be Source0: http://including a URL to download original
> tarball
Source come direct from Mandriva svn where we developed last years since the author, Gustavo Niemeyer left. So today tarball need be generated from there.

> 3) Spec missing %changelog section.
Fixed. 
Old habits from Mandriva where we have changelogs auto generated by buildsystem

> 4) Group should be Development/Tools
Fixed

> Is this your first package?    
For Fedora in a official way ? Yes
For my internal use ? No
First RPM.. nooooo,  a long noooo :-)

Hi Helio,
  it is good to see you here (as a kde user your name is quite familiar to me). :-)

Randall questions come from here
http://fedoraproject.org/wiki/PackageMaintainers/Join

So if this is your first package for Fedora the page advises:
http://fedoraproject.org/wiki/PackageMaintainers/Join#Create_Your_Review_Request

"You can add your package to this list by editing your review request bug and adding FE-NEEDSPONSOR in the 'Bug xyz blocks' field (where xyz is the bug number for your review request)."

Also since this is your first package you need to be sponsored in a process described here:

http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_Sponsored

FWIW I am a sponsor and I can drive this process if you want and Randal does not oppose.

Notice that I have reset the assignee role since for reviews this is reserved for the reviewer and not for the submitter (since this field is already set to you).

If you have any questions do not hesitate to ask. :-)

Hey, thanks

Rex Dieter already is my sponsor i think :-)

Most of the steps on the page for Join is already done, i have account. etc.. etc..

It's just time until i become adequate to the process.

José,
By all means feel free to take the reins. I'm not that familiar with packaging python (yet) so I'll just sit back and observe. My comments were just things I noticed as a passer by.

Hints for Python packages: 

 https://fedoraproject.org/wiki/Packaging:Python

As a start the --record=INSTALLED_FILES trick is not allowed, *.pyo files are not excluded and defattr use seems strange.

A link to a koji scratch build would be nice.

Done.

Modified the package this time following the fedora python package guide.
Here's the koji entry

http://koji.fedoraproject.org/koji/taskinfo?taskID=2373182

Thanks

Each time you make a change you have to note the change in the changelog section with version and release, bump the release up one, rebuild the SRPM and post your new spec and new SRPM in the review.

An example:

* Thu Jul 29 2010 Helio Chissini de Castro <helio> 2.2-2
- Edit spec per review.
- Rebuild SRPM

* Thu Jul 29 2010 Helio Chissini de Castro <helio> 2.2-1
- First release of bm based on mandriva release.

> Modified the package this time following the fedora python package guide.
> Here's the koji entry
> 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=2373182

Thanks. Better, however still som issues

 - please post direct links to updates spec and srpm, this make it much simpler
to review your package
 - python_sitearch macro is not used and should be dropped
 - I don't understand why recompiling pyc files are needed, seems to work fine without
 - the recipe for getting the sources must be more explicit, it must be possible
   to redo the source tarball down to the same bits, that means using a revision
   number from svn repo. See 
   https://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
   for more information.

All done
Links:
http://heliocastro.fedorapeople.org/bm-2.2-1.fc13.src.rpm
http://heliocastro.fedorapeople.org/bm.spec

Helio, here are some more things to consider:

- If you want to build the package for Fedora only (and not for EPEL), you can remove the BuildRoot tag (and also 'rm -rf %{buildroot}' in %install) [1]. Otherwise, it's recommended to use one of the tags listed at [2].

- you can drop BR: python (it's added automatically as a dependency)

- add %dir %{_sysconfdir}/bash_completion.d/ to %files
  (the package should own this directory, see [3])


[1] http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

[2] http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag

[3] http://fedoraproject.org/wiki/Packaging/Guidelines#Multiple_packages_own_files_in_a_common_directory_but_none_of_them_needs_to_require_the_others.

helio, I can pick this review up.

OH boy, I suck, forgot to follow up with anything useful here.  will try to do so this weekend.

I suck, I'll leave this for others at least I can dedicate enough time to do more reviews properly.

Is there a more recent package than the one in comment 10?  Any response from the submitter to comment 11?