Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 192546
| Summary: | Review Request: gnubiff | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Damien Durand <splinux> | ||||
| Component: | Package Review | Assignee: | Brian Pepple <bdpepple> | ||||
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | rawhide | CC: | gauret | ||||
| Target Milestone: | --- | ||||||
| Target Release: | --- | ||||||
| Hardware: | All | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2006-05-24 04:11:53 UTC | Type: | --- | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 163779 | ||||||
| Attachments: |
|
||||||
|
Description
Damien Durand
2006-05-20 11:36:25 UTC
Brian, are you reviewing this package ? Anyway here's my take at it (can't harm
to have more people for review)
Needs work:
* BuildRequires: gettext is missing (required by the %find_lang macro)
* /usr/share/info/dir is already owned by info, don't own it (put
%{_datadir}/info/*.info.gz in %files for example)
* Scriptlets: missing "install-info" in %post and %preun (in the wiki:
ScriptletSnippets)
Minor:
* Version and Source1 are not properly lined-up (tabs instead of spaces)
* Duplicate BuildRequires: gtk2-devel (by libglade2-devel)
* At the end of ./configure, there is "Gnome support: no". Is that what you want
? From http://gnubiff.sourceforge.net, GNOME support could be useful. Perhaps a
missing "BuildRequires: gnome-panel-devel" only ?
Ok, i've made the changes. Source : http://glive.tuxfamily.org/fedora/gnubiff/ Created attachment 129786 [details]
Mock build log failure
Your package fails to build in Mock. Also, you can drop the BR on
gettext-devel, since the default build environment in Mock installs gettext.
Change available at http://glive.tuxfamily.org/fedora/gnubiff/ Everything looks OK, but how about enabling GNOME support ? It looks like it can be embedded in the panel this way. It'd be a nice feature to have IMHO. By the way, version 2.2.1 is out. (In reply to comment #5) > Everything looks OK, but how about enabling GNOME support ? It looks like it can > be embedded in the panel this way. It'd be a nice feature to have IMHO. > > By the way, version 2.2.1 is out. > In addition to the GNOME support, it looks like it also has some SSL/crypto support that would be worthwile to enable. - Upgrade to 2.2.1 - Add --prefix='pkg-config libpanelapplet-2.0 openssl --variable=prefix in %con$ - Add gnome-panel-devel, openssl-devel in BuildRequires Changes available: http://glive.tuxfamily.org/fedora/gnubiff/ MD5Sums:
8d2ef679f42e7a593dc88b750d0cca4c gnubiff-2.2.1.tar.gz
Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* All directories are owned by this or other packages
* All necessary BuildRequires listed.
* All desired features are enabled
* Package builds in Mock.
* Package installs and uninstalls cleanly on FC5.
* rpmlint produces no error.
Bad:
* Don't pass '--prefix=`pkg-config libpanelapplet-2.0 openssl
--variable=prefix`' to the %configure macro. It's not needed.
Minor:
* In the file section, '%{_datadir}/info' should be '%{_infodir}'. Refer to
http://fedoraproject.org/wiki/Extras/RPMMacros
* Unnecessary documentation: ABOUT-NLS & Changelog. The first is a generic
build tools file, and the second is duplicate information that is included in
the NEWS file.
Once these items are fixed, considered this approved.
+1 Approve
- Remove --prefix='pkg-config libpanelapplet-2.0 openssl --variable=prefix OK
- Remove ABOUT-NLS & Changelog in %file section OK
- Fixing %{_datadir}/info to {_infodir} OK
Package imported in Extras
|