https://bugzilla.redhat.com/show_bug.cgi?id=1018492
Ankur Sinha (FranciscoD) <sanjay.an...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|fedora-review? |fedora-review+
--- Comment #9 from Ankur Sinha (FranciscoD) <sanjay.an...@gmail.com> ---
Review:
[+] OK
[-] NA
[?] Issue
** Mandatory review guidelines: **
[+] rpmlint output:
[asinha@ankur-laptop SRPMS]$ rpmlint
/var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
vcsh.noarch: W: spelling-error %description -l en_US config -> con fig,
con-fig, configure
vcsh.noarch: W: spelling-error %description -l en_US configs -> con figs,
con-figs, configure
vcsh.src: W: spelling-error %description -l en_US config -> con fig, con-fig,
configure
vcsh.src: W: spelling-error %description -l en_US zsh -> sh, ssh, ash
vcsh.src: W: spelling-error %description -l en_US configs -> con figs,
con-figs, configure
vcsh.src:52: W: macro-in-%changelog %{buildroot}
2 packages and 0 specfiles checked; 0 errors, 6 warnings.
[asinha@ankur-laptop SRPMS]$
[+] License is acceptable
[+] License field in spec is correct
[+] License files included in package %docs if included in source package
[+] License files installed when any subpackage combination is installed
[+] Spec written in American English
[+] Spec is legible
[+] Sources match upstream unless altered to fix permissibility issues
[asinha@ankur-laptop SRPMS]$ review-md5check.sh ../SPECS/vcsh.spec
Getting https://github.com/RichiH/vcsh/archive/v1.20130909.tar.gz to
/tmp/review/v1.20130909.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 124 100 124 0 0 56 0 0:00:02 0:00:02 --:--:-- 56
100 23850 100 23850 0 0 6317 0 0:00:03 0:00:03 --:--:-- 30228
6007dba2d65db1a8fe32d22d28094fd5 /tmp/review/v1.20130909.tar.gz
6007dba2d65db1a8fe32d22d28094fd5
/home/asinha/rpmbuild/SOURCES/v1.20130909.tar.gz
removed ‘/tmp/review/v1.20130909.tar.gz’
removed directory: ‘/tmp/review’
[asinha@ankur-laptop SRPMS]$
[+] Build succeeds on at least one primary arch
[+] Build succeeds on all primary arches or has ExcludeArch + bugs filed
[+] BuildRequires correct, justified where necessary
[-] Locales handled with %find_lang, not %_datadir/locale/*
[-] %post, %postun call ldconfig if package contains shared .so files
[+] No bundled libs
[-] Relocatability is justified
[+] Package owns all directories it creates
[+] Package requires others for directories it uses but does not own
Note that the package owns the /usr/share/zsh directory which is owned by other
packages also. However, since this is for optional functionality, this is OK:
https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function
[asinha@ankur-laptop SRPMS]$ sudo repoquery -l zsh | egrep '^/usr/share/zsh$'
/usr/share/zsh
[asinha@ankur-laptop SRPMS]$ rpmls
/var/lib/mock/fedora-rawhide-x86_64/result/vcsh-1.20130909-2.fc21.noarch.rpm
-rwxr-xr-x /usr/bin/vcsh
drwxr-xr-x /usr/share/doc/vcsh
-rw-r--r-- /usr/share/doc/vcsh/CONTRIBUTORS
-rw-r--r-- /usr/share/doc/vcsh/LICENSE
-rw-r--r-- /usr/share/doc/vcsh/README.md
-rw-r--r-- /usr/share/doc/vcsh/changelog
-rw-r--r-- /usr/share/doc/vcsh/hooks
-rw-r--r-- /usr/share/man/man1/vcsh.1.gz
drwxr-xr-x /usr/share/zsh
drwxr-xr-x /usr/share/zsh/site-functions
-rw-r--r-- /usr/share/zsh/site-functions/_vcsh
[asinha@ankur-laptop SRPMS]$
[+] No duplication in %files unless necessary for license files
[+] File permissions are sane
[+] Package contains permissible code or content
[-] Large docs go in -doc subpackage
[+] %doc files not required at runtime
[-] Static libs go in -static package/virtual Provides
[-] Development files go in -devel package
[-] -devel packages Require base with fully-versioned dependency, %_isa
[-] No .la files
[-] GUI app uses .desktop file, installs it with desktop-file-install
[+] File list does not conflict with other packages' without justification
[+] File names are valid UTF-8
** Optional review guidelines: **
[-] Query upstream about including license files
[-] Translations of description, summary
[+] Builds in mock
[+] Builds on all arches
[+] Functions as described (e.g. no crashes)
[-] Scriptlets are sane
[-] Subpackages require base with fully-versioned dependency if sensible
[-] .pc file subpackage placement is sensible
[+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
[+] Include man pages if available
Naming guidelines:
[+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
[+] Package names are sane
[+] No naming conflicts
[+] Spec file name matches base package name
[+] Version is sane
[+] Version does not contain ~
[+] Release is sane
[+] %dist tag
[-] Case used only when necessary
[-] Renaming handled correctly
Packaging guidelines:
[+] Useful without external bits
[+] No kmods
[-] Pre-built binaries, libs removed in %prep
[+] Sources contain only redistributable code or content
[+] Spec format is sane
[+] Package obeys FHS, except libexecdir, /run, /usr/target
[+] No files in /bin, /sbin, /lib* on >= F17
[-] Programs run before FS mounting use /run instead of /var/run
[-] Binaries in /bin, /sbin do not depend on files in /usr on < F17
[+] No files under /srv, /opt, /usr/local
[+] Changelog in prescribed format
[+] No Packager, Vendor, Copyright, PreReq tags
[+] Summary does not end in a period
[-] Correct BuildRoot tag on < EL6
[-] Correct %clean section on < EL6
[+] Requires correct, justified where necessary
[+] Summary, description do not use trademarks incorrectly
[+] All relevant documentation is packaged, appropriately marked with %doc
[+] Doc files do not drag in extra dependencies (e.g. due to +x)
[+] Code compilable with gcc is compiled with gcc
[+] Build honors applicable compiler flags or justifies otherwise
[-] PIE used for long-running/root daemons, setuid/filecap programs
[+] Useful -debuginfo package or disabled and justified
[-] Package with .pc files Requires pkgconfig on < EL6
[+] No static executables
[+] Rpath absent or only used for internal libs
[-] Config files marked with %config(noreplace) or justified %config
[+] No config files under /usr
[-] Third party package manager configs acceptable, in %_docdir
[-] .desktop files are sane
[+] Spec uses macros consistently
[+] Spec uses macros instead of hard-coded names where appropriate
[+] Spec uses macros for executables only when configurability is needed
[-] %makeinstall used only when alternatives don't work
[+] Macros in Summary, description are expandable at srpm build time
[+] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
[+] No software collections (scl)
[-] Macro files named /etc/rpm/macros.%name
[-] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
[-] %global, not %define
[-] Package translating with gettext BuildRequires it
[-] Package translating with Linguist BuildRequires qt-devel
[-] File ops preserve timestamps
[+] Parallel make
[+] No Requires(pre,post) notation
[-] User, group creation handled correctly (See Packaging:UsersAndGroups)
[-] Web apps go in /usr/share/%name, not /var/www
[-] Conflicts are justified
[+] One project per package
[+] No bundled fonts
[-] Patches have appropriate commentary
[-] Available test suites executed in %check
[-] tmpfiles.d used for /run, /run/lock on >= F15
Nitpicks:
- Please comment the patch, and send it upstream too.
- Since you're installing to pkgdocdir, and the %doc macro already takes
ownership of it, you don't need to specify it again. From
http://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25files_prefixes:
"Usually, "%doc" is used to list documentation files within %{_builddir} that
were not copied to %{buildroot}. A README and INSTALL file is usually included.
They will be placed in an appropriate directory under /usr/share/doc, whose
ownership does not need to be declared. "
Not blockers though. You can fix them up before you commit to SCM.
+++ APPROVED +++
Thanks,
Warm regards,
Ankur
--
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
