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 - Review Request: cssed - CSS editor and validator
Summary: Review Request: cssed - CSS editor and validator
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michał Bentkowski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
: 174063 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-06 22:59 UTC by Rafał Psota
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-01-10 15:14:35 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Rafał Psota 2007-01-06 22:59:45 UTC
Spec URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed.spec
SRPM URL: http://rafalzaq.nonlogic.org/fedora/cssed/cssed-0.4.0-1.fc6.src.rpm
Description: cssed is a small developer editor and validator, that tries to ease the CSS
editing.

It features syntax highlighting, syntax validation, MDI notebook based
interface, quick CSS properties and values insertion, auto-completion and
dialog-based insertion of CSS complex values.

Being a CSS editor, it's not limited to this language. cssed haved some
support for HTML (with embbeded Javascript), XML, Javascript, Java, PHP, JSP,
C, C++, Apache configuration files, .htaccess, Python, Perl, SQL, SH and other
languages so it can serve quite well as multi-purpose editor.

rpmlint returns only W: cssed-devel no-documentation.

There is a review request already but it's inactive (bug 174063).

This is one of my first packages so I'm looking for a sponsor.

Comment 1 Michał Bentkowski 2007-01-07 13:10:23 UTC
*** Bug 174063 has been marked as a duplicate of this bug. ***

Comment 2 Michał Bentkowski 2007-01-07 13:22:07 UTC
I'll review it.

Comment 3 Michał Bentkowski 2007-01-07 14:03:06 UTC
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

Comment 4 Rafał Psota 2007-01-07 14:17:40 UTC
(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

Comment 5 Michał Bentkowski 2007-01-07 16:53:58 UTC
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.

Comment 6 Rafał Psota 2007-01-07 17:59:58 UTC
(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

Comment 7 Michał Bentkowski 2007-01-07 18:29:36 UTC
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.

Comment 8 Mamoru TASAKA 2007-01-07 18:32:27 UTC
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"

Comment 9 Mamoru TASAKA 2007-01-07 18:40:16 UTC
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.

Comment 10 Mamoru TASAKA 2007-01-07 18:48:47 UTC
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.

Comment 11 Rafał Psota 2007-01-07 21:50:10 UTC
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

Comment 12 Mamoru TASAKA 2007-01-08 14:18:33 UTC
Okay, all I pointed out is now fixed and I can say
that this package can be imported to Fedora Extras.


Note You need to log in before you can comment on or make changes to this bug.