--- /home/adamw/public_html/reviews/openqa/openqa.spec	2016-02-04 13:34:16.640938990 -0800
+++ ./openqa.spec	2016-02-05 08:28:50.151479578 -0800
@@ -40,10 +40,9 @@
 
 Name:           openqa
 Version:        %{github_version}
-Release:        6%{?github_date:.%{github_date}git%{shortcommit}}%{?dist}
-Summary:        The openQA web-frontend, scheduler and tools
+Release:        7%{?github_date:.%{github_date}git%{shortcommit}}%{?dist}
+Summary:        OS-level automated testing framework
 License:        GPLv2+
-Group:          Development/System
 Url:            http://os-autoinst.github.io/openQA/
 Source0:        https://github.com/%{github_owner}/%{github_name}/archive/%{github_commit}/%{github_name}-%{github_commit}.tar.gz
 # Backport: don't slow things down by using dbus for thumbnails
@@ -55,6 +54,7 @@
 # Backport: fix ISO downloading when ISOURL is specified but not ISO
 # https://github.com/os-autoinst/openQA/pull/520
 Patch2:         0001-job_schedule_iso-fix-when-ISOURL-is-set-but-not-ISO.patch
+Obsoletes:      openqa < 4.3-7
 Requires(post): sqlite
 # This is for tests, currently disabled
 #BuildRequires: os-autoinst
@@ -63,7 +63,6 @@
 BuildRequires:  rubygem(sass)
 # needed for openid support
 Requires:       perl(LWP::Protocol::https)
-Requires:       httpd
 Recommends:     logrotate
 # for postgresql
 Recommends:     perl(DateTime::Format::Pg) perl(DBD::Pg)
@@ -106,32 +105,55 @@
 %requires_eq    perl-Mojolicious-Plugin-Bootstrap3 perl-Mojolicious-Plugin-AssetPack
 
 %description
-The openQA framework consists of two parts, which are tracked in
-separate git repos. The OS-autoinst test engine and the front-end
-with web-interface, test-scheduler and other high-level logic, which
-is part of this repo.
+openQA is a framework for operating system-level automated testing.
+It consists of three parts. This package contains the web interface,
+API, and scheduler. The worker component, which supervises test
+execution, is available in the openqa-worker package. The test engine,
+os-autoinst, is available in the os-autoinst package.
+
 
 %package common
-Summary:        The openQA common tools for web-frontend and workers
-Group:          Development/System
+Summary:        Common components for openQA server and workers
 Requires:       %{t_requires}
 
 %description common
-This package contain shared resources for openQA web-frontend and 
+This package contains shared resources for the openQA server and
 openQA workers.
 
+
 %package worker
 Summary:        The openQA worker
-Group:          Development/System
 Requires:       openqa-common = %{version}-%{release}
 Requires(post): coreutils
-Requires(post): os-autoinst >= 4
+Requires:       os-autoinst >= 4
 Requires:       os-autoinst < 5
 Requires:       qemu
 Requires:       perl(SQL::SplitStatement)
 
 %description worker
-The openQA worker manages test engine (provided by os-autoinst package).
+The openQA worker manages the os-autoinst test engine. A system with
+openqa-worker installed can run an arbitrary number of openQA workers
+(as many as its hardware can support), each of which will run a single
+openQA test job at a time, as long as appropriate jobs for the worker
+are available from the server it is configured to work for.
+
+
+%package httpd
+Summary:        openQA httpd (Apache) integration
+Requires:       httpd
+Requires:       httpd-filesystem
+# prior to 4.3-7, these files were part of the core package; this is
+# here so people who had those versions installed don't lose their
+# bits on update
+Obsoletes:      openqa < 4.3-7
+
+%description httpd
+This package contains httpd (Apache) configuration for the openQA
+automated testing framework. openQA runs as a self-contained http
+server which is expected to be reverse-proxied by a public-facing http
+server (rather than being accessed directly). The config snippets in
+this package help you configure openQA to be reverse proxied by httpd.
+
 
 %prep
 %autosetup -p1 -n %{github_name}-%{github_commit}
@@ -155,6 +177,8 @@
 # we don't really need the tidy test
 rm -f t/00-tidy.t
 #
+# tests are currently disabled on Fedora due to missing dependencies
+#
 #make test
 #prove -r
 
@@ -176,7 +200,7 @@
 ./script/openqa version  -m production
 cp -a public/packed %{buildroot}%{_datadir}/openqa/public/
 
-cd %buildroot
+cd %{buildroot}
 grep -rl /usr/bin/env . | while read file; do
   sed -e 's,/usr/bin/env perl,/usr/bin/perl,' -i $file
 done
@@ -209,36 +233,36 @@
 
 %post
 # install empty log file (see above for reasoning)
-if [ ! -e /var/log/openqa ]; then
+if [ ! -e %{_localstatedir}/log/openqa ]; then
         install -D -m 644 -o geekotest /dev/null %{_localstatedir}/log/openqa || :
 fi
 
 # do database
 if [ $1 -eq 1 ]; then
-        /usr/share/openqa/script/initdb --user geekotest --init_database || :
+        %{_datadir}/openqa/script/initdb --user geekotest --init_database || :
 
         echo "### copy and edit /etc/httpd/conf.d/openqa.conf.template!"
 else
-        /usr/share/openqa/script/upgradedb --user geekotest --upgrade_database || :
+        %{_datadir}/openqa/script/upgradedb --user geekotest --upgrade_database || :
 fi
 
-%systemd_post %openqa_services
+%systemd_post %{openqa_services}
 
 %post worker
 %tmpfiles_create %{_tmpfilesdir}/openqa.conf
-%systemd_post %openqa_worker_services
+%systemd_post %{openqa_worker_services}
 
 %preun
-%systemd_preun %openqa_services
+%systemd_preun %{openqa_services}
 
 %preun worker
-%systemd_preun %openqa_worker_services
+%systemd_preun %{openqa_worker_services}
 
 %postun
-%systemd_postun_with_restart %openqa_services
+%systemd_postun_with_restart %{openqa_services}
 
 %postun worker
-%systemd_postun_with_restart %openqa_worker_services
+%systemd_postun_with_restart %{openqa_worker_services}
 
 %files
 %doc README.asciidoc docs/*
@@ -252,12 +276,6 @@
 %{_datadir}/openqa/etc/openqa/database.ini
 %config %{_sysconfdir}/logrotate.d
 %config(noreplace) %{_sysconfdir}/dbus-1/system.d/org.opensuse.openqa.conf
-# apache vhost
-%dir %{_sysconfdir}/httpd
-%dir %{_sysconfdir}/httpd/conf.d
-%config %{_sysconfdir}/httpd/conf.d/openqa.conf.template
-%config %{_sysconfdir}/httpd/conf.d/openqa-common.inc
-%config %{_sysconfdir}/httpd/conf.d/openqa-ssl.conf.template
 # init
 %dir %{_unitdir}
 %{_unitdir}/openqa-webui.service
@@ -299,7 +317,6 @@
 %ghost %{_localstatedir}/log/openqa
 
 %files common
-%{!?_licensedir:%global license %doc}
 %license COPYING
 %dir %{_datadir}/openqa
 %{_datadir}/openqa/lib
@@ -329,7 +346,24 @@
 # own one pool - to create the others is task of the admin
 %dir %{_localstatedir}/lib/openqa/pool/1
 
+%files httpd
+%license COPYING
+# apache vhost
+%dir %{_sysconfdir}/httpd
+%dir %{_sysconfdir}/httpd/conf.d
+%config %{_sysconfdir}/httpd/conf.d/openqa.conf.template
+%config %{_sysconfdir}/httpd/conf.d/openqa-common.inc
+%config %{_sysconfdir}/httpd/conf.d/openqa-ssl.conf.template
+
 %changelog
+* Fri Feb 05 2016 Adam Williamson <awilliam@redhat.com> - 4.3-7
+- package review improvements:
+- * no need for worker to Requires(post) os-autoinst
+- * explain why tests are currently disabled
+- * fix a few macro invocations to use curly braces
+- * use directory macros where appropriate in scriptlets
+- * split apache configuration into a subpackage
+
 * Thu Jan 28 2016 Adam Williamson <awilliam@redhat.com> - 4.3-6
 - update ISOURL patch to latest revision