User:Gholms/review template

From FedoraProject

< User:Gholms(Difference between revisions)
Jump to: navigation, search
(Added perl guidelines)
Line 1: Line 1:
 +
This page contains copypasteable templates for package reviews.  The [[#Core Guidelines|Core Guidelines]] section contains the rules that apply to every package.  The rest contain rules that apply to specific types of packages.  Those can be appended to the core guidelines.
 +
 +
== Core Guidelines ==
 
<pre>
 
<pre>
 
Mandatory review guidelines:
 
Mandatory review guidelines:
  - rpmlint output
+
  - rpmlint output:
 
   ...
 
   ...
- Package meets naming guidelines
 
- Spec file name matches base package name
 
 
  - License is acceptable (...)
 
  - License is acceptable (...)
 
  - License field in spec is correct
 
  - License field in spec is correct
  - License files included in package %docs or not included in upstream source
+
  - License files included in package %docs if included in source package
 
  - License files installed when any subpackage combination is installed
 
  - License files installed when any subpackage combination is installed
 
  - Spec written in American English
 
  - Spec written in American English
 
  - Spec is legible
 
  - Spec is legible
 
  - Sources match upstream unless altered to fix permissibility issues
 
  - Sources match upstream unless altered to fix permissibility issues
   Upstream MD5: ...
+
   Upstream MD5: ...
   Your MD5:     ...
+
   Your MD5:     ...
  - Build succeeds on at least one supported platform
+
  - Build succeeds on at least one primary arch
  - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
+
  - Build succeeds on all primary arches or has ExcludeArch + bugs filed
 
  - BuildRequires correct
 
  - BuildRequires correct
  - Package handles locales with %find_lang
+
  - Locales handled with %find_lang, not %_datadir/locale/*
 
  - %post, %postun call ldconfig if package contains shared .so files
 
  - %post, %postun call ldconfig if package contains shared .so files
  - No bundled system libs
+
  - No bundled libs
 
  - Relocatability is justified
 
  - Relocatability is justified
 
  - Package owns all directories it creates
 
  - Package owns all directories it creates
  - Package requires other packages for directories it uses but does not own
+
  - Package requires others for directories it uses but does not own
  - No duplicate files in %files unless necessary for license files
+
  - No duplication in %files unless necessary for license files
 
  - File permissions are sane
 
  - File permissions are sane
  - Each %files section contains %defattr on EL4
+
  - Package contains permissible code or content
- Consistent use of macros
+
  - Large docs go in -doc subpackage
- Sources contain only permissible code or content
+
  - %doc files not required at runtime
  - Large documentation files go in -doc package
+
  - Static libs go in -static package/virtual Provides
  - Missing %doc files do not affect runtime
+
  - Development files go in -devel package
- Headers go in -devel package
+
  - -devel packages Require base with fully-versioned dependency, %_isa
  - Static libs go in -static package
+
  - No .la files
  - Unversioned .so files go in -devel package
+
  - GUI app uses .desktop file, installs it with desktop-file-install
  - Devel packages require base with fully-versioned dependency
+
  - File list does not conflict with other packages' without justification
  - Package contains no .la files
+
  - GUI app uses desktop-file-install/desktop-file-validate for .desktop files
+
  - Package's files and directories don't conflict with others' or justified
+
 
  - File names are valid UTF-8
 
  - File names are valid UTF-8
  
 
Optional review guidelines:
 
Optional review guidelines:
 
  - Query upstream about including license files
 
  - Query upstream about including license files
  - Translations of description, Summary
+
  - Translations of description, summary
 
  - Builds in mock
 
  - Builds in mock
  - Builds on all supported platforms
+
  - Builds on all arches
  - Functions as described
+
  - Functions as described (e.g. no crashes)
 
  - Scriptlets are sane
 
  - Scriptlets are sane
  - Non-devel subpackage Requires are sane
+
  - Subpackages require base with fully-versioned dependency if sensible
  - .pc files go in -devel unless main package is a development tool
+
  - .pc file subpackage placement is sensible
  - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
+
  - No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
  - Man pages included for all executables
+
  - Include man pages if available
  - Package with test-suite executes it in %check section
+
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:
 
Packaging guidelines:
- Has dist tag
 
 
  - Useful without external bits
 
  - Useful without external bits
  - Package obeys FHS, except libexecdir, /usr/target, /run
+
- No kmods
  - No file in /bin, /sbin, /lib* on >= F17
+
- Pre-built binaries, libs removed in %prep
  - Programs launched before FS mounting use /run instead of /var/run
+
- 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
 
  - Binaries in /bin, /sbin do not depend on files in /usr on < F17
 +
- No files under /srv, /opt, /usr/local
 
  - Changelog in prescribed format
 
  - Changelog in prescribed format
  - Spec file lacks Packager, Vendor, PreReq tags
+
  - No Packager, Vendor, Copyright, PreReq tags
  - Correct BuildRoot tag on < F10/EL6
+
- Summary does not end in a period
  - Correct %clean section on < F13/EL6
+
  - Correct BuildRoot tag on < EL6
 +
  - Correct %clean section on < EL6
 
  - Requires correct, justified where necessary
 
  - Requires correct, justified where necessary
 +
- BuildRequires correct, justified where necessary
 
  - Summary, description do not use trademarks incorrectly
 
  - Summary, description do not use trademarks incorrectly
  - All relevant documentation is packaged, tagged appropriately
+
  - All relevant documentation is packaged, appropriately marked with %doc
  - Documentation files do not have executable permissions
+
  - Doc files do not drag in extra dependencies (e.g. due to +x)
 
  - Code compilable with gcc is compiled with gcc
 
  - Code compilable with gcc is compiled with gcc
  - %build honors applicable compiler flags or justifies otherwise
+
  - Build honors applicable compiler flags or justifies otherwise
 
  - PIE used for long-running/root daemons, setuid/filecap programs
 
  - PIE used for long-running/root daemons, setuid/filecap programs
- Package with .pc files Requires pkgconfig on < EL6
 
 
  - Useful -debuginfo package or disabled and justified
 
  - Useful -debuginfo package or disabled and justified
 +
- Package with .pc files Requires pkgconfig on < EL6
 
  - No static executables
 
  - No static executables
 
  - Rpath absent or only used for internal libs
 
  - Rpath absent or only used for internal libs
  - Config files marked with %config
+
  - Config files marked with %config(noreplace) or justified %config
- %config files marked noreplace or justified
+
  - No config files under /usr
  - No %config files under /usr
+
 
  - Third party package manager configs acceptable, in %_docdir
 
  - Third party package manager configs acceptable, in %_docdir
  - Spec uses macros instead of hard-coded directory names where appropriate
+
- .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
 
  - Spec uses macros for executables only when configurability is needed
  - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
+
  - %makeinstall used only when alternatives don't work
  - Macros in Summary, %description expandable at SRPM build time
+
  - Macros in Summary, description are expandable at srpm build time
  - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
+
  - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
 
  - No software collections (scl)
 
  - No software collections (scl)
  - Build uses only python/perl/shell+coreutils/lua/BuildRequired languages
+
  - Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
  - %global instead of %define where appropriate
+
  - %global, not %define
  - Package containing translations BuildRequires gettext
+
  - Package translating with gettext BuildRequires it
  - File timestamps preserved by file ops
+
- Package translating with Linguist BuildRequires qt-devel
 +
  - File ops preserve timestamps
 
  - Parallel make
 
  - Parallel make
  - Spec does not use Requires(pre,post) notation
+
  - No Requires(pre,post) notation
 
  - User, group creation handled correctly (See Packaging:UsersAndGroups)
 
  - User, group creation handled correctly (See Packaging:UsersAndGroups)
  - Web app files go in /usr/share/%{name}, not /var/www
+
  - Web apps go in /usr/share/%name, not /var/www
 
  - Conflicts are justified
 
  - Conflicts are justified
- No external kernel modules
 
- No files in /srv, /opt, /usr/local
 
 
  - One project per package
 
  - One project per package
  - Patches link to upstream bugs/comments/lists or are otherwise justified
+
- No bundled fonts
  - Packages needing dirs in /var/run or /var/lock use tmpfiles.d on >= F15
+
  - Patches have appropriate commentary
 +
  - Available test suites executed in %check
 +
- tmpfiles.d used for /run, /run/lock on >= F15
 +
</pre>
  
Application / Language-specific guidelines:
+
== SysV Init Script Guidelines ==
  ...
+
<pre>
 +
  - Init scripts go in /etc/rc.d/init.d
 +
- Init scripts not marked with %config
 +
- Init script configuration in /etc/sysconfig
 +
- Init scripts have 0755 permissions
 +
- Packages with unit files put init scripts in -sysvinit subpackage
 +
- chkconfig, initscripts Requires, init scripts correct
 +
- Init scripts have chkconfig headers
 +
- Init script environment variables have reasonable defaults
 +
- Init scripts implement all required actions
 +
- Init script behavior is sensible
 +
- Init script return codes are correct
 
</pre>
 
</pre>
  
Line 162: Line 190:
 
<pre>
 
<pre>
 
  - Runtime Requires correct
 
  - Runtime Requires correct
  - Python macros declared on < F13/EL6
+
  - Python macros declared on < EL6
 
  - All .py files packaged with .pyc, .pyo counterparts
 
  - All .py files packaged with .pyc, .pyo counterparts
 
  - Includes .egg-info files/directories when generated
 
  - Includes .egg-info files/directories when generated

Revision as of 21:59, 29 June 2012

This page contains copypasteable templates for package reviews. The Core Guidelines section contains the rules that apply to every package. The rest contain rules that apply to specific types of packages. Those can be appended to the core guidelines.

Contents

Core Guidelines

Mandatory review guidelines:
 - rpmlint output:
   ...
 - 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
   Upstream MD5: ...
   Your MD5:     ...
 - Build succeeds on at least one primary arch
 - Build succeeds on all primary arches or has ExcludeArch + bugs filed
 - BuildRequires correct
 - 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
 - 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
 - BuildRequires 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)
 - 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

SysV Init Script Guidelines

 - Init scripts go in /etc/rc.d/init.d
 - Init scripts not marked with %config
 - Init script configuration in /etc/sysconfig
 - Init scripts have 0755 permissions
 - Packages with unit files put init scripts in -sysvinit subpackage
 - chkconfig, initscripts Requires, init scripts correct
 - Init scripts have chkconfig headers
 - Init script environment variables have reasonable defaults
 - Init scripts implement all required actions
 - Init script behavior is sensible
 - Init script return codes are correct

Systemd Guidelines

 - Traditional service uses a unit file
 - Non-standard service commands converted to standalone scripts
 - Unit names are sane
 - Description= lines do not exceed 80 characters
 - Types are correct
 - Requires=, Wants= used only when necessary
 - Units to not refer to runlevel*.target
 - Symlinks used instead of Name=
 - StandardOutput=, StandardError= used only when necessary
 - Socket-activated service has FESCo approval, correct unit files
 - Unit files go in %_unitdir
 - BuildRequires: systemd-units for %_unitdir macro
 - Packaged unit files are not %config files
 - Unit file scriptlets are correct
 - tmpfiles.d used where needed

Java Guidelines

 - Javadocs go in javadoc subpackage
 - Prefer split JARs over monolithic
 - JAR file names correct
 - JAR files go in %{_javadir} or %{_javadir}-$version
 - Multiple JAR files go in a %{name} subdirectory
 - Javadocs go in unversioned %{_javadocdir}/%{name}
 - javadoc subpackage is noarch on > EL5
 - BuildRequires java-devel, jpackage-utils
 - Requires java >= $version, jpackage-utils
 - Dependencies on java/java-devel >= 1.6.0 add epoch 1
 - Package requiring maven2 Requires jpackage-utils for post and postun
 - Package requiring maven contains correct maven-specific code in spec
 - Wrapper script in %{_bindir}
 - GCJ AOT bits follow GCJ guidelines
 - No devel package
 - pom.xml files, if any, installed with %add_maven_depmap
 - JNI shared objects, JARs that require them go in %{_libdir}/%{name}
 - Calls to System.loadLibrary replaced w/ System.load w/ full .so path
 - Bundled JAR files not included or used for build
 - No Javadoc %post/%ghost
 - No class-path elements in JAR manifests

Perl Guidelines

 - Module requirements use virtual perl(modname) syntax
 - Spec BuildRequires correct core modules, not perl-devel
 - Spec contains correct MODULE_COMPAT Requires
 - Requires/Provides are sane
 - CPAN URL tag is not versioned
 - All tests enabled where possible
 - Use Build.PL if present unless justified otherwise
 - .h files not split into -devel package

Python Guidelines

 - Runtime Requires correct
 - Python macros declared on < EL6
 - All .py files packaged with .pyc, .pyo counterparts
 - Includes .egg-info files/directories when generated
 - Provides/Requires properly filtered
 - Code that invokes gtk.gdk.get_pixels_array() Requires numpy