[Bug 518636] Review Request: django-reversion - Django extension that provides version control capabilities

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=518636

--- Comment #8 from Michel Alexandre Salim <fedora@xxxxxxxxxxxxxxxxxx> 2010-11-13 09:32:18 EST ---
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

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]