[Bug 993] Review request: openshot - GNOME Non-linear video editor

[RPM Fusion Bugzilla]

http://bugzilla.rpmfusion.org/show_bug.cgi?id=993





--- Comment #77 from Hans de Goede <j.w.r.degoede at 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.