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

--- Comment #122 from Christoph Wickert <[email protected]> ---
Some hints purely from looking at the spec and not the srpm:

- Don't use %define, consistently use %global
- Don't use fuzzy patches but rebase them
- Don't use the %{version} in the name of patches because it means you need to
rename the patch when you package a version even if it still applies cleanly.
The version in the patch name should be hardcoded and refer to the version
where a patch was introduced.
- Patches should have comments and links to upstream or downstream bugs so we
can easily see if something is being upstreamed or not.
- First apply upstream patches, than downstream ones. Start downstream ones at
say %patch10 or %patch20, then you have enough room for upstream fixes.
- Some of the Requires and BuildRequires seem redundant
- Better use install rather than cp because to make sure permissions are
correct and timestamps are preserved.
- use "make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'" tp preserve
timestamps
- Don't hardcode directory names like /usr/share in the sed commands, use
macros
- The glib-compile-schemas scriptlets are wrong, they don't handle the
upgrading. Please use the ones form the wiki.
- The package provides a dbus service, but does not require dbus-x11. How is
dbus started then?
- Does cinnamon provide a polkit-authentication agent (password entry dialog)?
If so, add Provides: PolicyKit-authentication-agent, if not, require it.
- Why do you move the cinnamon settings menu entry to the "Utilities" group?
- Why do you remove the included *.session and xsession files and provide your
own as additional sources? Please add a comment as explanation.
- Don't specify the manpage with full extension, use %{name}.1.* instead of
%{name}.1.gz in case we switch to another compression method.
- Why not simply own %{_datadir}/cinnamon/ ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to