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 171565 - Review Request: drgeo
Summary: Review Request: drgeo
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gérard Milmeister
QA Contact: David Lawrence
URL: http://www.ofset.org/drgeo
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-10-23 10:28 UTC by Eric Tanguy
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-10-25 04:16:00 UTC
Type: ---
Embargoed:

Attachments (Terms of Use)

Description Eric Tanguy 2005-10-23 10:28:06 UTC 
Spec Name or Url: http://perso.wanadoo.fr/eric.tanguy/drgeo.spec
SRPM Name or Url: http://perso.wanadoo.fr/eric.tanguy/drgeo-1.1.0-1.src.rpm
Description: Dr. Geo is a interactive geometry GUI application. It allows one to create geometric figure plus the interactive manipulation of such figure in respect with their geometric constraints. It is usable in teaching situation with students from primary or secondary level.
Comment 1 Gérard Milmeister 2005-10-23 13:43:19 UTC 
Spec File:
* Url: http://www.ofset.org/drgeo
* %description: "is a interactive" -> "is an interactive"
  "figure" -> "figures"
* BuildRequires: why include gnome-libs-devel, which is for gnome 1.4?
* "%configure" suffices no need for "--prefix"
* I think "rm" and "make" can (should?) be used instead of the macros
* A litte post-processing of the .desktop file in %install:
  desktop-file-install \
    --delete-original \
    --vendor=fedora \
    --add-category X-Fedora \
    --dir %{buildroot}%{_datadir}/applications \
    %{buildroot}%{_datadir}/applications/drgeo.desktop
  Of course, BuildRequires: desktop-file-utils
* .desktop file sets icon to gnome-drgenius.png which does not exist.
  Suggest copying drgeo.png and setting "Icon: drgeo.png"
  (why is this commented out?)
* Since there is no html documentation, consider patching the source
  to remove the "Contents" menu and button, and notifying upstream
  to correct this.
* The texmacs files should go to %{_datadir}/TeXmacs/plugins/drgeo
* update changelog
Comment 2 Eric Tanguy 2005-10-23 17:52:04 UTC 
(In reply to comment #1)
> Spec File:
> * Url: http://www.ofset.org/drgeo

Done

> * %description: "is a interactive" -> "is an interactive"
>   "figure" -> "figures"

Done

> * BuildRequires: why include gnome-libs-devel, which is for gnome 1.4?
> * "%configure" suffices no need for "--prefix"

Done

> * I think "rm" and "make" can (should?) be used instead of the macros

Done

> * A litte post-processing of the .desktop file in %install:
>   desktop-file-install \
>     --delete-original \
>     --vendor=fedora \
>     --add-category X-Fedora \
>     --dir %{buildroot}%{_datadir}/applications \
>     %{buildroot}%{_datadir}/applications/drgeo.desktop
>   Of course, BuildRequires: desktop-file-utils

Done

> * .desktop file sets icon to gnome-drgenius.png which does not exist.
>   Suggest copying drgeo.png and setting "Icon: drgeo.png"
>   (why is this commented out?)

Done

> * Since there is no html documentation, consider patching the source
>   to remove the "Contents" menu and button, and notifying upstream
>   to correct this.

Ok I notified it upstream but i know only few about programming and i don't how
to patch this ...

> * The texmacs files should go to %{_datadir}/TeXmacs/plugins/drgeo

Done

> * update changelog

Done

> 
> 

Spec Name or Url: http://perso.wanadoo.fr/eric.tanguy/drgeo.spec
SRPM Name or Url: http://perso.wanadoo.fr/eric.tanguy/drgeo-1.1.0-2.src.rpm
Comment 3 Gérard Milmeister 2005-10-23 19:29:45 UTC 
* replace the remaining %{__rm} and %{__install} by rm and install
* in the changelog you used 1.0.1 instead of 1.1.0

Make these small fixes and everything is ok.

APPROVED
Comment 4 Paul Howarth 2005-10-24 07:19:44 UTC 
(In reply to comment #3)
> * replace the remaining %{__rm} and %{__install} by rm and install

Why? In what way does this improve the package? This is largely a cosmetic issue
but if anything I would say that using the macros was better, since they expand
to fully-qualified pathnames and hence don't make the result of the build
dependent on the value of the building user's PATH setting.
Note You need to log in before you can comment on or make changes to this bug.