[Bug 506833] Review Request: bisho - Moblin web services settings

[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=506833





--- Comment #5 from Peter Robinson <pbrobinson at gmail.com>  2009-08-06 19:12:34 EDT ---
I'd already fixed most of these locally yesterday based on the other reviews
but hasn't uploaded it yet.

> Issues:
> - Fix the license tag

Fixed

> - Drop 'Requires(post): /bin/touch', explained in bug 507480

Done

> - Drop the redundant BuildRequires, it's no use listing them: glib2-devel,
> pkgconfig are pulled in be nearly every devel package, autoconf and automake
> are required by libtool.

I don't see what is the major issue, dependencies change over time and it
doesn't add build time so it ends up being semantics. I have removed them. 

> - The comment "Require these because ..." is misleading. gnome-common is
> (likely) needed and gettext/intltool are needed because of the locales. So all
> that is actually required to run autogen.sh is libtool. Please change the
> comment to reflect this.

Well none of them would be needed at all if the package was a released package
that had "make dist" run, hence the comment. Maybe I haven't documented it
properly. gnome-common isn't pulled in automatically hence the reason it was
added, nor are gettext/intltool I believe.

> - Timestamps are nor preserved during %install, add "INSTALL='install -p'" to
> make install, see
> https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
> - Update icon-cache scriptlet with latest version from
> https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

Done.

> - What libtool archives are you trying to remove? There are none!

Copied over from a template, removed.

> - AUTHORS and TODO are missing from %doc. Don't add NEWS and README (empty) or
> ChangeLog (not useful)

Added, they use to be empty :)

> - Use desktop-file-install or desktop-file-validate for the desktop file, see
> https://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage

Updated.

> Please fix the issues and contact rel-eng in order to get mux into the repo.
> Once this is done, I will finish this review.  

Not an issue, mux was in rawhide for about 2 weeks and then marked as a
dead.package due to being merged into nbtk.

Also fixed up the autoconf.sh so it doesn't run configure twice.

SRPM: http://pbrobinson.fedorapeople.org/bisho-0.10.7-2.fc11.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1588011

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