This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 171565 - Review Request: drgeo
Review Request: drgeo
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Gérard Milmeister
David Lawrence
http://www.ofset.org/drgeo
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-10-23 06:28 EDT by Eric Tanguy
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-10-25 00:16:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Eric Tanguy 2005-10-23 06:28:06 EDT
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 09:43:19 EDT
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 13:52:04 EDT
(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 15:29:45 EDT
* 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 03:19:44 EDT
(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.