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

[RPM Fusion Bugzilla]

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





--- Comment #79 from Richard <hobbes1069 at gmail.com>  2011-04-10 18:14:39 ---
(In reply to comment #77)
> 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.

Fixed


> - 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

Sorry, I didn't think it was necessary to bump the release until it became a
"real" package. Fixed.


> - 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

I haven't needed it yet but the original spec author could comment. I'll remove
it for now.

Technically per the Fedora packaging guidelines BuildRoot is no longer needed.
Should I remove it as well?


> - "CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build" should be just
>   "%{__python} setup.py build" specifying CFLAGS for a noarch package is weird.

It was probably left over from the spec template since it assumes it is arch
specified I believe. Fixed.


> - 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.

Done.

I'm rebuilding now to make sure I didn't muck anything up!

Richard


-- 
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.