[Bug 2366] Review request: mythweb

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=2366

Richard <hobbes1069 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #3 from Richard <hobbes1069 at gmail.com> 2012-07-01 16:53:23 CEST ---
(In reply to comment #2)
> Must: Please add a %build section. Interestingly your spec crashed
> fedora-review :) https://bugzilla.redhat.com/836772

Should we add something just to placate fedora-review? As far as I know it
isn't strictly needed, unless there's a guideline I missed? It could be added
manually just to fix fedora-review (which I assume you did) but I'd rather not
put it in permanently if there's no requirement to do so.


> Must: Please fix the directory permissions. Something like this should do the
> trick: find . -type d -print0 | xargs -0 chmod 755 

Strange, this seems to have been done on purpose by Jarod. I'm not sure why.
%defattr(-,apache,apache,0775)
%{_datadir}/mythweb/

I took it out. I don't think it should break anything.


> Must: Please adjust the licensing, since it differs slightly from the main
> MythTV package. From what I see it is just "GPLv2 and LGPLv2 and MIT". 

Done.


> Nice-to-have items, that do not block approval, in order of preference:
> 
> You may want to put a note in the comments on how to generate
> mythweb-fixes.patch. I imagine it's just a "git diff TAG1..TAG2" thing?

Added... Since HEAD is assumed I use:
git diff -p --stat %{version} > mythweb-fixes.patch


> I'm not sure mythtv-backend is a strict requirement, since technically the web
> interface could run on a separate box from the backend server. Same for the
> Requires: mysql-server.

Yeah, here I have to choose from the "best compromise" situation since RPM
fails us here. People who run  separate FE/BE configurations are more likely to
know what they're doing so and can probably build MythTV from source. I suppose
though that people who use the everything install via the mythtv meta package
are covered. I'll think about it.


> Maybe add a note in the comments to give a hint for what "githash3" and
> "githash4" are. Also, now that the package is split, can these variables be
> renamed to "githash1" and "githash2"?

Done.


> This is just a minor style thing, but httpd 1.3 is very ancient. It would be
> cleaner to remove the specific httpd version number and say "Require: httpd".

Done.


> If you want to silence the non-executable-script warning, you can use sed in
> %prep:  sed -i modules/coverart/handler.pl -e '/\/usr\/bin\/perl/d'

Done.


SPEC: https://dl.dropbox.com/u/34775202/mythweb/mythweb.spec
SRPM: https://dl.dropbox.com/u/34775202/mythweb/mythweb-0.25.1-2.fc16.src.rpm

-- 
Configure bugmail: https://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.