[Bug 594839] Review Request: kpartsplugin - KParts technology to embed file viewers into non-KDE browsers

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=594839

--- Comment #2 from Thomas Janssen <thomasj at fedoraproject.org> 2010-05-26 16:32:16 EDT ---
Hi Magnus, thanks for the review!

> * MUST: The package must be licensed with a Fedora approved license and meet
> the Licensing Guidelines.
> - OK: BSD
> 
> * MUST: The License field in the package spec file must match the actual
> license.
> - NOT OK: License is GPLv3+ according to $homepage (GPL), and README.txt
> (GPLv3+).

Seems i confused two packages. FIXED

> * MUST: A package must own all directories that it creates. If it does not
> create a directory that it uses, then it should require a package which does
> create that directory.
> - NOT SURE

see below

> * MUST: Packages must not own files or directories already owned by other
> packages. The rule of thumb here is that the first package to be installed
> should own the files or directories that other packages may rely upon. This
> means, for example, that no package in Fedora should ever share ownership with
> any of the files or directories owned by the filesystem or man package. If you
> feel that you have a good reason to own a file or directory that another
> package owns, then please present that at package review time.
> - NOT SURE:
> - %files section has
> %{_kde4_libdir}/nsbrowser
> but I think
> %{_kde4_libdir}/nsbrowser/plugins/libkpartsplugin.so
> is more appropriate.
> * SHOULD: The reviewer should test that the package functions as described. A
> package should not segfault instead of running, for example.
> - NOT OK, firefox did not find the plugin.
> mock build fedora-13-x86_64.
> However, firefox found the plugin after "cd /usr/lib64/mozilla/plugins/ && ln
> -s /usr/lib64/nsbrowser/plugins/libkpartsplugin.so"

I changed the install path to ../mozilla/plugins/ and added mozilla-filesystem
to requires for directory ownership. FIXED

Both oversights well spotted. Thank you.

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