[Bug 993] Review request: openshot - GNOME Non-linear video editor
http://bugzilla.rpmfusion.org/show_bug.cgi?id=993
--- Comment #77 from Hans de Goede <j.w.r.degoede(a)gmail.com> 2011-04-10 11:52:40
---
Hi,
I've finally managed to make time to do a full review of this, below are the
results:
Good:
=====
- rpmlint checks return:
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
- package meets naming guidelines
- package meets packaging guidelines
- license ( ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
Needs work:
===========
- The %if ! (0%{?fedora} > 12 || 0%{?rhel} > 5) .. %endif block at the top
should be dropped IMHO, as I don't expect openshot to ever get supported
on EL-5, and Fedora 12 is end of life already.
- I notice that Release: is still one. Next time you do an update (like
the changing of $RPM_BUILD_ROOT vs. %{buildroot}) please bump Release
and add a %changelog entry with the changes. Within Fedora we've the rule
to bump Release and add a %changelog entry even during review to make it
easier for reviewers to check what has changed
- If this is no longer needed please drop it:
# Fix documentation
#for file in `find docs/gnome/ -name '*.xml'`; do
#sed -i -e 's~<\(caption\|mediaobject\|emphasis\)/>~~g' -e '/^[
]\+$/d' $fi
#done
- "CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build" should be just
"%{__python} setup.py build" specifying CFLAGS for a noarch package is weird.
- Please add a comment above "License GPLv3", that all files are GPLv3+, except
for files under openshot/uploads/youtube which are "ASL 2.0" and
openshot/window/SimpleGtkBuilderApp.py which is "LGPLv3", making the
effective
license of openshot GPLv3.
The needs work items, are really just small nitpicks / small fixes. So good
job!
Regards,
Hans
--
Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.