A brief look at the spec file (no full review):


> #==================
> # Macro definitions
> #==================
> %global url          http://codemirror.net/
> %global pub          http://aikiframework.org/files/codemirror/
> %global name         codemirror
> %global version      0.91
> %global revision     1
> %global pkgdist      %{name}-%{version}.zip
> %global pkgdistdir   CodeMirror-%{version}
> %global httpdconf    %{name}.conf
> %global httpdconfdir %{_sysconfdir}/httpd/conf.d
> %global httpdservice /sbin/service httpd
> 
> #====================
> # Package definitions
> #====================
> Name:      %{name}
> Version:   %{version}
> Release:   %{revision}%{?dist}
> Summary:   In-browser code editing made bearable
> Group:     Applications/Internet
> License:   zlib
> URL:       %{url}
> Source0:   %{url}/%{pkgdist}
> Source1:   %{httpdconf}
> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> BuildArch: noarch

In my opinion, these two sections are a superfluous overuse of variables. I highly recommend trimming down the spec preamble to improve readability and to remove ambiguities.

The spec file is "huge enough" because of many subpackages that during update preparations it will be necessary to spend time on (re-reviewing) the individual subpackage definition sections anyway (e.g. the various licences and URLs). Having to deal with macros that are used only once, e.g. %pub, isn't helpful. The various licenses could be summed up near the top of the spec, btw.


> %global name         codemirror
> Name:      %{name}

The "Name:" tag already defines %name, so what you do here is redundant. Simply

  Name: codemirror

near the top of the spec file would be very clear and sufficient.


> %global version      0.91
> Version:   %{version}

Same here. Plus, you'll get into trouble whenever you will need to apply special pre-release/post-release versioning schemes (such as Fedora's).


> %global revision     1
> Release:   %{revision}%{?dist}

Worse even. It would be necessary to modify both lines for a least-significant %release bump, such as  2%{?dist}.1  Apart from that, %revision is not used elsewhere, so why introduce it at all? "Release:" already defines %release.


> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Note that several details related to BuildRoot are not needed anymore in Fedora 13 and newer: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> #=============
> # Requirements
> #=============
> Requires: httpd

The fat header is superfluous. Better add a brief comment that explains this explicit dependency on "httpd", and possibly answer why a dependency on the virtual "webserver" package wouldn't work.

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

That Wiki section focuses on library Requires a bit, but also applies to explicit Requires in general.


> %package doc
> Summary:  CodeMirror documentation
> Group:    Documentation
> Requires: %{name} = %{version}-%{release}

Here, a comment would explain why a Documentation package needs the base package. I see the some of the HTML pages refer to .js files on localhost, so that's sort of what a comment in the spec should cover.


> %{__mkdir} -p -m 0755 %{buildroot}%{_defaultdocdir}/%{name}-%{version}
> %{__install} -p -m 0644 *.html %{buildroot}%{_defaultdocdir}/%{name}-%{version}

Caution! Manually filling %_defaultdocdir/%{name}-%{version} with files conflicts with %doc. It's a pitfall that packagers run into regularly during review. If in a later release, a %doc line is added to the files list for %name, it would lead to killing/overriding the contents of the manually filled directory. Possible work-arounds: 1) Using a different docdir. 2) Adding a warning to the %files list that %doc MUST NOT be used.


> #=============
> # Post process
> #=============
> %post
> %{httpdservice} condrestart &> /dev/null || :
> %postun
> %{httpdservice} condrestart &> /dev/null || :
> 
> #==========================
> # Files included in package
> #==========================

These fat headers here bite you. Examine the output of:
rpm -qp --scripts codemirror-0.91-1.fc14.noarch.rpm

This has bitten other packagers before, even worse when using non-Shell scriptlet sections.


> $ rpmls -p codemirror-0.91-1.fc14.noarch.rpm|grep ^d
> $

https://fedoraproject.org/wiki/Packaging:UnownedDirectories

For example, directory entries /usr/share/codemirror/ and /usr/share/doc/codemirror-0.91/ are not included in the package. Mistake may apply to other subpackages, too.


> # http://svn.exoplatform.org/projects/gwt/trunk/exo-gwtframework-editor/src/main/resources/org/exoplatform/gwtframework/editor/public/codemirror/css/groovycolors.css
> Source4:  groovycolors.css

Wouldn't it be much more convenient to use the full download URL directly in the Source tag? Especially if you don't rename the file. I see that in some of the sources, you rename the downloaded file to avoid conflicts. How about adding a tiny shell script that as a separate Source file, which can be run to automate the wget downloading and renaming of all Source files you need? And optionally show a diff compared with previously download files. It would make package maintenance easier.

Hmm, no response to the above commentary in 16 months.  I'm just going to close this.