[Bug 991639] Review Request: python-facct - Fig's accountancy tool

[bugzilla at redhat.com]

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



--- Comment #5 from efigue <efigue.foss at free.fr> ---
(In reply to Michael Schwendt from comment #3)
> Confirmed. Macro-madness is not a blocker according to the guidelines, but
> all those macros will cause you some headaches eventually. I won't approve a
> package that redefines several implicitly defined macros. Examples:
> 
> > %define version 0.1.2
> 
> The "Version" tag already defines %{version}, so replace
> 
>   Version: %{version}
> 
> with
> 
>   Version: 0.1.2
> 
> and you can use %{version} anywhere you like.
> 
> 
> > %define release 1
> 
> This is wrong and misleading. Here you assign the value 1 to %release, but
> further down in the spec you do
>  
>   Release: %{release}%{?dist}
> 
> which defines %{release} to this new value. The old definition is lost.
> Mass-rebuild scripts also will have a lot of fun trying to figure out
> whether to modify (= "bump") the Release tag or the definition of "release".
> Please don't make it so complicated.
> 
> 
> > Url: %{url}
> 
> "URL: http://efigue.foss.free.fr" and you're done. Well, you reuse %{url}
> for the Source0 tag, but hey, the "URL" tag defines %{url}, too. This is not
> an obfuscation contest, but a rather simple package which should come with a
> rather simple and readable spec file.
> 
> 
> >  %define name python-facct
> 
> Again, the "Name" tag defines %{name}. There is really no need to redefine
> these macros at the top of the spec file, becoming a crowded place, and you
> would need to scroll back'n'forth to locate where and how those macros are
> used throughout the spec file.
> 
> 
> * Run rpmlint (or rpmlint -I for more helpful output) on the src.rpm and all
> built rpms. Feel free to ignore obvious false positives in the report, but
> fix
> anything else. Preferably add a comment here about whether/when you think
> what
> rpmlint reports is correct or incorrect.
> 
>   $ rpmlint python-facct-0.1.2-1.fc19.src.rpm 
>   python-facct.src: W: file-size-mismatch facct-0.1.2.tar.gz = 316035,
> http://efigue.foss.free.fr/facct-0.1.2.tar.gz = 316042
>   1 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 
> 
> > %files -f %{_tmppath}/INSTALLED_FILES
> > %files -f %{srcname}.lang
> 
> This is severely flawed. Just query the package contents to verify what is
> included currently:
> 
>   $ rpmls -p python-facct-0.1.2-1.fc19.noarch.rpm|grep ^d
>   drwxr-xr-x  /usr/lib/python2.7/site-packages/facct-0.1.2-py2.7.egg-info
>   drwxr-xr-x  /usr/share/doc/python-facct-0.1.2
> 
> Directories are missing!
> https://fedoraproject.org/wiki/Packaging:UnownedDirectories
> 
> There are also duplicate files in both packages, e.g.:
> 
> $ rpmls -p python3-facct-0.1.2-1.fc19.noarch.rpm | grep locale
> -rw-r--r-- 
> /usr/lib/python2.7/site-packages/facct/locale/fr_FR/LC_MESSAGES/facct.mo
> -rw-r--r-- 
> /usr/lib/python3.3/site-packages/facct/locale/fr_FR/LC_MESSAGES/facct.mo
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files

Thanks for the review and explanations.
I uploaded a new version also for the size check.

Regards

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=G6MVtwrEIh&a=cc_unsubscribe