Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 221727
Summary: | Review Request: cssed - CSS editor and validator | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rafał Psota <rafalzaq> |
Component: | Package Review | Assignee: | Michał Bentkowski <mr.ecik> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora, mtasaka |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-10 15:14:35 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Rafał Psota
2007-01-06 22:59:45 UTC
*** Bug 174063 has been marked as a duplicate of this bug. *** I'll review it. REVIEW: * rpmlint gives me following result: W: cssed-devel no-documentation can be ignored because of lack of any devel-related documentation * I have no objection to name of package and spec file * package is licensed under a GPL license and the license file is included into %doc * package builds fine in mock fc6/x86_64 * sources match upstream (md5: ff7c818d1f819b7d76b4f714be64e08e) * locales are handled well * all directories are properly owned !* %clean section presents but looks bad, you should change it to rm -rf %{buildroot} * %defattr macro presents in all %files sections * macros used well * -devel subpackage presents and looks good !* bad icon tag in desktop-file THINGS to do: * change %clean section * you have used cssed.png entry in desktop.file but that file doesn't exist. It seems to me that the best way would be changing it to /usr/share/cssed/pixmaps/cssed-about.png (In reply to comment #3) > THINGS to do: > * change %clean section > * you have used cssed.png entry in desktop.file but that file doesn't exist. > It seems to me that the best way would be changing it to > /usr/share/cssed/pixmaps/cssed-about.png Fixed. Spec URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed.spec SRPM URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed-0.4.0-2.fc6.src.rpm It's better to check rpmlint again before sending an each new release. If you made that, you would see following result: W: cssed macro-in-%changelog clean It means you have to double % character before clean, so write %%clean instead of %clean in changelog. (In reply to comment #5) > It's better to check rpmlint again before sending an each new release. If you > made that, you would see following result: > W: cssed macro-in-%changelog clean > It means you have to double % character before clean, so write %%clean instead > of %clean in changelog. Fixed. Spec URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed.spec SRPM URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed-0.4.0-3.fc6.src.rpm Okay, now it does look fine. Package is approved. I'll sponsor you. Just request your cvsextras and fedorabugs account. After then you'll be able to import and build your package. Read http://fedoraproject.org/wiki/Extras/Contributors carefully to know what you're to do. If package builds fine, don't forget to close that ticket. Well, I have to make some comments: * Please check the requirement for -devel package. cssed.pc includes the line: ----------------------------------------------- Requires: gtk+-2.0 glib-2.0 ----------------------------------------------- This means that -devel package needs "Requires: gtk2-devel" Also, ----------------------------------------------- Cflags: -I${prefix}/share/cssed/include ----------------------------------------------- points to a wrong directory. Also this file has a strange coding at the end (try "cat cssed.pc"). * Please avoid to use autotools when it is possible. It seems that from your patch against Makefile.am and the content of Makefile.in, you can make a patch against Makefile.in, not against Makefile.am with ease. * Please remove redundant BuildRequires. For example, glib2-devel is always required by gtk2-devel, so writting "glib2-devel" Also: * some header files have CRLF (i.e. Windows-like) end-of-line encodings and these should be fixed. * fedora-cssed.desktop includes: ------------------------------------------- Categories=Application;Development;TextEditor;X-Fedora; ------------------------------------------- Two categories "Application" "X-Fedora" is now deprecated and so these should be removed. Also one issue. /usr/include/cssed/SciLexer.h includes: ---------------------------------------------- // Copyright 1998-2002 by Neil Hodgson <neilh> // The License.txt file describes the conditions under which this software may be distributed. ---------------------------------------------- So please include scintilla/License.txt Note: this is MIT? If so, the license of this "whole" package is still GPL. I fixed all these things. Spec URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed.spec SRPM URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed-0.4.0-4.fc6.src.rpm Okay, all I pointed out is now fixed and I can say that this package can be imported to Fedora Extras. |