Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: gtksourceview https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225875 roozbeh@xxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|roozbeh@xxxxxxxxxxxxx |rstrode@xxxxxxxxxx CC| |mclasen@xxxxxxxxxx Flag|fedora-review? |fedora-review- ------- Additional Comments From roozbeh@xxxxxxxxxxxxx 2007-02-05 07:47 EST ------- Doing a thorough review this time. BAD === MUST: License * The license file in the package says GPL, while the license tag in the spec says LGPL. Some file headers say GPL, while others say LGPL, which makes the whole package GPL. So spec file should be changed to say GPL. (blocker) * As some of the files are LGPL-ed, a copy of the LGPL should also be included in the upstream tarball and also shipped in the RPM. Contact upstream (SHOULD item). * Some files like gtksourceview-marshal.c and gtksourceview-typebuiltins.c don't have a license header, which makes them proprietary (pessimist view) or at least unknown licensing (considering that some files in the same dir are GPL while others are LGPL. No trivial fix, but contacting upstream is recommended again. MUST: owning dirs * -devel package puts files in %{_datadir}/gtk-doc/html, while not depending on any packages that owns that dir. I don't know who should be the lucky package, but this is a blocker anyway. MUST: build requires * As configure.in requires intltool 0.35.0, which is not available on FC5, the package should have a versioned build dependency on intltool >= 0.35.0 (blocker). IMPROVEMENTS ============ Grammer * I guess gtksourceview should be changed to GtkSourceView in the description of the -devel package (second line), as it's refering to the library/widget, not the package. Docs * Consider marking files under %{_datadir}/gtk-doc/ as %doc Style * Add extra slash to the end of directories in the %files section when you want to include their files also: %{_datadir}/gtksourceview-1.0/ %{_includedir}/gtksourceview-1.0/ %{_datadir}/gtk-doc/html/gtksourceview/ GOOD ==== MUST: rpmlint output W: gtksourceview-devel no-documentation MUST: package naming fine MUST: spec name matches package name MUST: US English fine MUST: packaging guidelines followed MUST: free software MUST: tarball's license is shipped in RPM as %doc MUST: spec file legible MUST: tarball matches upstream (md5sum checked) MUST: built binary RPMs on FC6/i386 MUST: no ExcludeArch MUST: locales handled properly MUST: ldconfig used properly MUST: not relocatable MUST: no dups in %files MUST: file permissions fine MUST: %clean fine MUST: macro use consistent MUST: has code MUST: no large docs MUST: header files in -devel, no static libs MUST: -devel has *.pc and Requires pkgconfig MUST: *.so goes in -devel MUST: -devel has fully versioned dep on main MUST: *.la removed explicitly MUST: not GUI MUST: doesn't own others' files SHOULD: no scriptlets -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review