Spec URL: http://heliocastro.fedorapeople.org/bm.spec SRPM URL: http://heliocastro.fedorapeople.org/bm-2.2-1.fc13.src.rpm Description: bm, also known as BuildManager is a handy tool to build inplace rpms using a local rpm structure, without the need to be root or an specific environment setup for user, with ability to create logs of every operation performed. It comes with a handy extra macros for use --without as a better bypass short-circuit during the development and test. As an example, "bm -l --without build" will short-circuit the package to the install stage, without clean up the build dir, so test installs in large packages will reduce time when something failed in install stage.
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?