Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=946856 Tom "spot" Callaway <tcallawa@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |tcallawa@xxxxxxxxxx --- Comment #5 from Tom "spot" Callaway <tcallawa@xxxxxxxxxx> --- Hi Lokesh. Here are a few areas where I'd like to see changes in this spec file: * You've got a lot of unnecessary space in this spec file. Consider replacing all "double" line breaks with single line breaks, and removing the single empty lines in the top section (e.g. between Summary and License, between Patch0 and URL, between Source0 and BuildRequires). You also do not need newline space between the entries under %files. * Consider using the -b flag when applying patches to append a identifier string. It makes it easier to regenerate diffs in the future: %patch0 -p1 -b .foo * Please move scriptlets to the bottom of the spec file. This is a semantic thing, but I prefer the flow of the spec file to match the order of operations for the package build. They should go below %install, before %files. * You have an empty %doc entry. While there does not appear to be any documentation included with this package (not even a README or a license file, just the man pages), you don't need to have an empty %doc entry in the spec. * Be sure you are incrementing Release every time you make a change to the spec file, and adding a new changelog entry. * There seems to be a .desktop file for spectrwm, not sure if that makes sense to package or not. * The package generates a versioned shared library file, but it doesn't use -Wl,-soname,libspectrwm.so.0 Then, in addition to installing libspectrwm.so.0.0.0, you'll want to create symlinks to libspectrwm.so.0 and libspectrwm.so. The unversioned .so file should go in a -devel subpackage, along with the few .h files it pulls in. The devel subpackage needs to Require: %{name} = %{version}-%{release} * You should be using %{name} and %{version} anywhere it is hardcoded (except for the Name: and Version: fields, of course). Notably, in %files and Source entries. Show me a new spec file, and I should be able to finish off this review quickly (and sponsor you). -- 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=fptuW4F4sy&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review