Bug 234122
Summary: | Review Request: pygtkglext - Python bindings for GtkGLExt | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Hans de Goede <hdegoede> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | Flags: | mtasaka:
fedora-review+
wtogami: fedora-cvs+ |
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: | 2007-04-02 16:25:15 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: |
Description
Hans de Goede
2007-03-27 08:09:03 UTC
I will review this after I return to home. Well, for 1.1.0-1 * %makeinstall ------------------------------------- # Note: make install PREFIX=$RPM_BUILD_ROOT doesnot work :( ------------------------------------- - Perhaps you want to use the following? ------------------------------------- make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p" ------------------------------------- The above works for me. And INSTALL option should be added to keep timestamps on python scripts installed. * Documentation ------------------------------------- sed -i 's/\r//g' README.win32 ------------------------------------- - Simply remove this line. README._win32_ should not be for linux. * %files - By the way, why does file entry has the same line? (perhaps you just did copy & paste) * libtool archives - Perhaps .la files should be removed. * Executable permission vs shebang ------------------------------------- chmod +x $RPM_BUILD_ROOT%{python_sitearch}/gtk-2.0/gtk/gtkgl/apputils.py ------------------------------------- - Again please check if this is correct * Requires ------------------------------------- Requires: gtkglext ------------------------------------- - Is this needed? Libraries' dependency should automatically pull this. * python_sitelib vs python_sitearch ------------------------------------- if [ %{python_sitelib} != %{python_sitearch} ]; then <skip> fi ------------------------------------- - While this should work, IMO it is better to configure configure says: ------------------------------------- 19151 if test "${am_cv_python_pythondir+set}" = set; then 19152 echo $ECHO_N "(cached) $ECHO_C" >&6 19153 else 19154 am_cv_python_pythondir=`$PYTHON -c "from distutils import sysconfig; print sysconfig.get_python_lib(0,0,prefix='$PYTHON_PREFIX')" 2>/dev/null || 19155 echo "$PYTHON_PREFIX/lib/python$PYTHON_VERSION/site-packages"` 19156 fi ------------------------------------- From my impression, this should be okay with changing to get_python_lib(1,0,prefix='$PYTHON_PREFIX') * License - Well, the included files are GPL and LGPL, and actually this is licensed under LGPL (also declared on http://www.k-3d.org/gtkglext/Main_Page#Licensing ) * Encoding - The following documents are not encoded with UTF-8. Please fix. -------------------------------------- <file> <encoded by> AUTHORS EUC-JP README EUC-JP (README.win32 SJIS) -------------------------------------- (Just a note: usually Japanese documents are not encoded with UTF-8 and normally encoded with either EUC-JP, ISO-2022-JP <both are for UNIX file> or SJIS <for Windows file>) (In reply to comment #2) > * %files > - By the way, why does file entry has the same line? > (perhaps you just did copy & paste) > Those lines are not identical (gdkgl versus gtkgl) > * Executable permission vs shebang > ------------------------------------- > chmod +x $RPM_BUILD_ROOT%{python_sitearch}/gtk-2.0/gtk/gtkgl/apputils.py > ------------------------------------- > - Again please check if this is correct > Actually this file contains a main with test routines and thus can be executed stand alone, I've added a comment about this. > * python_sitelib vs python_sitearch > ------------------------------------- > if [ %{python_sitelib} != %{python_sitearch} ]; then > <skip> > fi > ------------------------------------- > - While this should work, IMO it is better to configure > configure says: > ------------------------------------- > 19151 if test "${am_cv_python_pythondir+set}" = set; then > 19152 echo $ECHO_N "(cached) $ECHO_C" >&6 > 19153 else > 19154 am_cv_python_pythondir=`$PYTHON -c "from distutils import sysconfig; > print sysconfig.get_python_lib(0,0,prefix='$PYTHON_PREFIX')" 2>/dev/null || > 19155 echo "$PYTHON_PREFIX/lib/python$PYTHON_VERSION/site-packages"` > 19156 fi > ------------------------------------- > From my impression, this should be okay with changing to > get_python_lib(1,0,prefix='$PYTHON_PREFIX') > Erm the above solution gives me a headache, and as said my own (simple, understandable) solution works fine. All other items fixed, here is a new version: Spec URL: http://people.atrpms.net/~hdegoede/pygtkglext.spec SRPM URL: http://people.atrpms.net/~hdegoede/pygtkglext-1.1.0-2.fc7.src.rpm Okay. -------------------------------------------- This package (pygtkglext) is APPROVED by me -------------------------------------------- (In reply to comment #3) > (In reply to comment #2) > > * %files > > - By the way, why does file entry has the same line? > > (perhaps you just did copy & paste) > > > Those lines are not identical (gdkgl versus gtkgl) Oops..... New Package CVS Request ======================= Package Name: pygtkglext Short Description: Python bindings for GtkGLExt Owners: j.w.r.degoede Branches: FC-6 devel InitialCC: <empty> Many thanks for the review! Imported and build, closing. (Actually closing) |