Spec URL: http://lucabotti.fedorapeople.org/packages/django-reversion.spec SRPM URL: http://lucabotti.fedorapeople.org/packages/django-reversion-1.1.2-1.fc11.src.rpm Description: Reversion is an extension to the Django web framework that provides comprehensive version control facilities. Features: * Roll back to any point in a model's history - an unlimited undo facility! * Recover deleted models - never lose data again! * Admin integration for maximum usability. * Group related changes into revisions that can be rolled back in a single transaction. * Automatically save a new version whenever your model changes using Django's flexible signalling framework. * Automate your revision management with easy-to-use middleware. Reversion can be easily added to your existing Django project with a minimum of code changes. See http://code.google.com/p/django-reversion/
Hello -- welcome to the world of Fedora packagers! Some preliminary suggestions -- full review to follow, after which I can sponsor you. If you could also provide links to reviews you have done, or other packaging work (preferably RPM-based), even if these are not part of any distribution, please do so. - for the Source0: field, you want to include the full URL. e.g. http://django-reversion.googlecode.com/files/%{name}-%{version}.tar.gz - URL at the end of %description is redundant; rpm -qi will show it - %setup -q is enough; it defaults to -n %{name}-%{version} - include release number in the Changelog. If you use Emacs, the easiest way to generate a changelog entry is to use M-x rpm-add-changelog-entry (you can tab-complete after rpm-add) Use rpmlint on the source and binary RPMs after each update to make sure there are no issues -- and let me know if there is anything unclear. e.g. using your spec, I get this: $ rpmlint ../RPMS/noarch/django-reversion-1.1.2-1.fc12.noarch.rpm django-reversion.noarch: W: incoherent-version-in-changelog 1.1.2 ['1.1.2-1.fc12', '1.1.2-1'] ==> missing revision number (you can skip the %{?dist} part) django-reversion.noarch: W: no-documentation ==> %files should have a %doc section. In this case, it appears the author did not provide any documentation at all, so you'd want to ask him to bundle a license file with a tarball. For now, you can package the PKG-INFO file, since it comes from upstream and at least mentions the BSD license (though it also says LICENSE UNKNOWN).
Hi Michel, thanks for suggestions. Here the new links: http://lucabotti.fedorapeople.org/packages/django-reversion-1.1.2-2.fc11.src.rpm http://lucabotti.fedorapeople.org/packages/django-reversion.spec Updated to -2 given the fixes in SPEC file. Thanks again and regards.
Hullo! Package looks good enough to normally pass review (though see the one recommended fix in the SHOULD section). Now I just need to see some more evidence of packaging knowledge (esp. since the initial package comes from Tim! Didn't know he does Python packages too, I thought it's only Lua). You cannot do full reviews, since you are not sponsored yet, but you can pre-review other packages -- see http://fedoraproject.org/wiki/PackageMaintainers/ReviewRequests for a list of unassigned reviews. Make sure you clearly state that it's a pre-review, and not assign the review to yourself (preventing an authorized reviewer from seeing it). Here's my review for this, as a reference: It's a bit more verbose than normal, I normally take out the irrelevant Not Applicable parts, but in case you review (or package) different types of software, they might come in handy. MUST OK rpmlint $ rpmlint ../SRPMS/django-reversion-1.1.2-2.fc12.src.rpm ../RPMS/noarch/django-reversion-1.1.2-2.fc12.noarch.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. OK package name OK spec file name OK package guideline-compliant OK license complies with guidelines ? license field accurate upstream is unclear about this. Project page and one mention in PKG-INFO states BSD, but there is no license file and PKG-INFO also lists license as UNKNOWN NA license file not deleted not included by upstream. See SHOULD section below OK spec in US English OK spec legible OK source matches upstream $ sha1sum django-reversion-1.1.2.tar.gz ../SOURCES/django-reversion-1.1.2.tar.gz 8ff80fb027dc8f98d21f479b19ef0b450b266811 django-reversion-1.1.2.tar.gz 8ff80fb027dc8f98d21f479b19ef0b450b266811 ../SOURCES/django-reversion-1.1.2.tar.gz OK builds under >= 1 archs, others excluded built using Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1717143 OK build dependencies complete NA locales handled using %find_lang, no %{_datadir}/locale NA library -> ldconfig NA relocatable: give reason OK own all directories OK no dupes in %files OK permission OK %clean RPM_BUILD_ROOT OK macros used consistently OK Package contains code NA large docs => -doc OK doc not runtime dependent NA headers in -devel NA static in -static NA if contains *.pc, req pkgconfig NA if libfiles are suffixed, the non-suffixed goes to devel NA devel requires versioned base package NA desktop file uses desktop-file-install OK clean buildroot before install OK filenames UTF-8 SHOULD FIX if license text missing, ask upstream to include it perhaps post a bug at the upstream tracker and then put a comment in the spec above the %doc PKG-INFO line? that way you can package the correct license file once an fixed update comes out NA desc and summary contain translations if available well, nice to have, but I've only seen one package that does this (and the upstream author added the translation). OK package build in mock on all architectures ? package functioned as described OK scriplets are sane NA other subpackages should require versioned base NA if main pkg is development-wise, pkgconfig can go in main package OK require package not files
Hi Michel, For the SHOULD part, I opened the issue with the author, which is going to add the documents for next point release, due in a couple of weeks. For the experience, i will take care of the pre-review part. For reference on other packaging, see the others packages submitted in bugzilla or go to my blog (http://www.lbotti.net/blog). Thanks again
Anything happening here?
Apologies; package fell through the crack. Do you still need sponsorship? In which case, since there is now a 1.3.2 release, please do a quick update and I'll verify that everything is (still) in order. https://github.com/etianen/django-reversion/wiki/getting-started
Hi Michel, this are the links for the latest SPEC and SRPMS http://lucabotti.fedorapeople.org/packages/django-reversion-1.3.2-1.fc14.src.rpm http://lucabotti.fedorapeople.org/packages/django-reversion.spec Thanks
Hi Luca, You don't need to define both python_sitelib and python_sitearch -- for noarch packages like this only python_sitelib is used. Also: license file needs to be included, and you need to handle locale files. See complete review below * TODO Review [40%] ** DONE Names [2/2] *** DONE Package name *** DONE Spec name ** DONE Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]] ** DONE source files match upstream e13db17c02693c136be0585d1b32b5a0 django-reversion-1.3.2.tar.gz e13db17c02693c136be0585d1b32b5a0 ../SOURCES/django-reversion-1.3.2.tar.gz ** TODO License [2/3] *** DONE License is Fedora-approved *** DONE License field accurate *** FAIL License included iff packaged by upstream please include LICENSE ** TODO rpmlint [1/2] *** DONE on src.rpm $ rpmlint django-reversion-1.3.2-1.fc14.src.rpm django-reversion.src: W: spelling-error %description -l en_US signalling -> sign aling, signal ling, signal-ling django-reversion.src: W: spelling-error %description -l en_US middleware -> midd le ware, middle-ware, middleweight 1 packages and 0 specfiles checked; 0 errors, 2 warnings. Warnings are just due to the spellchecker not understanding technical jargon *** FAIL on x86_64.rpm $ rpmlint ./django-reversion-1.3.2-1.fc14.noarch.rpm django-reversion.noarch: W: spelling-error %description -l en_US signalling -> signaling, signal ling, signal-ling django-reversion.noarch: W: spelling-error %description -l en_US middleware -> middle ware, middle-ware, middleweight Those can be ignored django-reversion.noarch: W: file-not-in-%lang /usr/lib/python2.7/site-packages/reversion/locale/de/LC_MESSAGES/django.mo django-reversion.noarch: W: file-not-in-%lang /usr/lib/python2.7/site-packages/reversion/locale/fr/LC_MESSAGES/django.mo django-reversion.noarch: W: file-not-in-%lang /usr/lib/python2.7/site-packages/reversion/locale/he/LC_MESSAGES/django.mo django-reversion.noarch: W: file-not-in-%lang /usr/lib/python2.7/site-packages/reversion/locale/it/LC_MESSAGES/django.mo django-reversion.noarch: W: file-not-in-%lang /usr/lib/python2.7/site-packages/reversion/locale/pl/LC_MESSAGES/django.mo django-reversion.noarch: W: file-not-in-%lang /usr/lib/python2.7/site-packages/reversion/locale/ru/LC_MESSAGES/django.mo 1 packages and 0 specfiles checked; 0 errors, 8 warnings. These, however, need to be handled. see %find_lang section below. ** TODO Language & locale [2/3] *** DONE Spec in US English *** DONE Spec legible *** FAIL Use %find_lang to handle locale files See for example how the main Django package handle this: # This is adapted from the %%find_lang macro, which cannot be directly # used since Django locale files are not located in %%{_datadir} # # The rest of the packaging guideline still apply -- do not list # locale files by hand! (cd $RPM_BUILD_ROOT && find . -name 'django*.mo') | %{__sed} -e 's|^.||' | %{__sed} -e \ 's:\(.*/locale/\)\([^/_]\+\)\(.*\.mo$\):%lang(\2) \1\2\3:' \ >> %{name}.lang In this case you can reuse this without modifcation, since reversion's .mo files are also named django.mo ** DONE Build [3/3] *** DONE Koji results *** DONE BRs complete *** DONE Directory ownership ** TODO Spec inspection [5/10] *** WAIT No duplicate files note that once you use the modified find_lang script, you need to make sure the language files are not listed manually in %files anymore. You can NOT use %exclude for this as it would actually remove the excluded files, not just avoid re-listing them, so instead you'd have to manually list all the other directories: %files -f %{name}.lang ... %dir %{python_sitelib}/reversion %{python_sitelib}/reversion/*.py* %{python_sitelib}/reversion/management %{python_sitelib}/reversion/templates %{python_sitelib}/reversion/templatetags *** DONE File permissions *** DONE Filenames must be UTF-8 *** DONE no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting EPEL5]]) *** DONE Has %clean section (except F-13+: https://fedoraproject.org/wiki/Packaging/Guidelines#.25clean) *** TODO %buildroot cleaned on %install *** WAIT Macro usage consistent several minor fixes: - python_sitearch is redundant - probably should use %{version} in the source field, that way when you update the package you don't need to change the version in two places *** DONE Documentation [2/2] **** N/A If large docs, separate -doc **** DONE %doc files are non-essential
Hi Michel, thanks for the review and the suggestions. You can find new version of spec and src.rpm in http://lucabotti.fedorapeople.org/packages/django-reversion.spec and http://lucabotti.fedorapeople.org/packages/django-reversion-1.3.2-2.fc14.src.rpm Regards
Hi Luca, Looks good -- with one minor glitch, that you can fix when importing the package: you have *two* %changelog section! Please rpmlint the .spec file when you make changes. Review is APPROVED, since the fix is trivial enough. And I've sponsored you. Please follow the next step in the review guidelines: 1. Change fedora-cvs flag to ? and create a new SCM request: http://fedoraproject.org/wiki/Package_SCM_admin_requests you only need to request the stable branches; 'master' targets Rawhide and you automatically get that For your next several packages, please put me in the InitialCc: list, and you're more than welcome to mail me with any packaging question or review requests. 2. Build package for Rawhide and other stable branches http://fedoraproject.org/wiki/Using_Fedora_GIT 3. Apart from el6 (which just went stable), the stable branches all required updates to be pushed through Bodhi; please create an update request for this package there: https://admin.fedoraproject.org/updates/new You can list the built packages for multiple distributions on the same form; they will share all properties (description, bugs closed, automatic push karma and automatic cancellation karma etc.) but will be partitioned based on the targeted release. Welcome to the packaging club :)
New Package SCM Request ======================= Package Name: django-reversion Short Description: Reversion is an extension to the Django web framework that provides comprehensive version control facilities. Owners: lucabotti Branches: f13 f14 el5 el6 InitialCC:
Cc:ing myself -- and removing new lines from the description :) New Package SCM Request ======================= Package Name: django-reversion Short Description: Reversion is an extension to the Django web framework that provides comprehensive version control facilities. Owners: lucabotti Branches: f13 f14 el5 el6 InitialCC: salimma
I wonder why you don't use the one-liner from the bug summary, and even that could be shorted to something like "Django extension to provide version control".
Just the emotion ;-) New Package SCM Request ======================= Package Name: django-reversion Short Description: django-reversion - Django extension that provides version control capabilities Owners: lucabotti Branches: f13 f14 el5 el6 InitialCC: salimma
You don't want the package name in the description either though, just the package summary.
Indeed, the "django-reversion - " part has to go, it should just be "Django ...".
That's it - lesson learned :-) New Package SCM Request ======================= Package Name: django-reversion Short Description: Django extension that provides version control capabilities Owners: lucabotti Branches: f13 f14 el5 el6 InitialCC: salimma
Git done (by process-git-requests).