[Bug 771252] Review Request: cinnamon - Window management and application launching for GNOME

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

 



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

--- Comment #122 from Christoph Wickert <cwickert@xxxxxxxxxxxxxxxxx> ---
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
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]