[Bug 555376] Review Request: lorem-ipsum-generator - Generates random lorem ipsum text

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--- Comment #5 from Jean-Francois Saucier <jfsaucier@xxxxxxxxxxxx> 2010-01-15 21:54:24 EST ---
Spec URL: http://jfsaucier.fedorapeople.org/packages/lorem-ipsum-generator.spec
SRPM URL:
http://jfsaucier.fedorapeople.org/packages/lorem-ipsum-generator-0.3-2.fc12.src.rpm


> - As Jussi pointed out, the files section should be more explicit

I think it's good now.


> - You are running the install command twice:

Fixed. Don't know how I miss this one.


> 1. There is no icon because there is no "python" icon

My fault, I have a theme that provide such an icon. I replaced it with the
generic gnome-mime-text-x-python since the package doesn't provide one.


> 2. Why is StartupNotify set to false?

I removed this from the desktop file. I based my desktop file from another
program and I forgot to remove this line before doing the patch.


> 3. Should IMHO have the additional category "TextTools" (if this is no text tool, what then?)

Yes, you are right! This tool fits right in that category. I added it to the
desktop file.


> - Hint: Patches are usually named %{name}-%{version}-what-it-does.patch

Changed. Thanks for pointing this out.


> One more thing: Please add TODO to the docs.

Added!


rpmlint give no warning/error on spec, srpm and rpm.

Build fine in my local mock instance.


Thank you for taking the time to review this package!

-- 
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.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]