Bug 701932

Summary: Review Request: emacs-pymacs - Emacs and Python integration framework
Product: [Fedora] Fedora Reporter: Stanislav Ochotnicky <sochotni>
Component: Package ReviewAssignee: Daiki Ueno <dueno>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dueno, fedora-package-review, notting
Target Milestone: ---Flags: dueno: fedora-review+
j: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-05-11 07:38:59 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Stanislav Ochotnicky 2011-05-04 11:00:41 UTC
Spec URL: http://sochotni.fedorapeople.org/packages/pymacs.spec
SRPM URL: http://sochotni.fedorapeople.org/packages/pymacs-0.23-1.fc14.src.rpm


Description: Pymacs is a powerful tool which, once started from Emacs, allows
both-way communication between Emacs Lisp and Python. Pymacs aims
Python as an extension language for Emacs rather than the other way
around, and this asymmetry is reflected in some design choices. Within
Emacs Lisp code, one may load and use Python modules. Python functions
may themselves use Emacs services, and handle Emacs Lisp objects kept
in Emacs Lisp space.

Comment 1 Daiki Ueno 2011-05-10 03:09:05 UTC
Hi, thanks for packaging this!  The Python packaging looks fine with me.  One suggestion is that the package should follow the Emacs packaging guideline, since the description says "once started from Emacs".  This sounds for me that the package is mainly used with Emacs.

https://fedoraproject.org/wiki/Packaging:Emacs

To migrate, the package will need to:
- be renamed or split from pymacs to emacs-pymacs
- byte-compile /usr/share/emacs/site-lisp/pymacs.el
- use %{_emacs_site*dir} macros
- add a startup file for autoloading

Comment 2 Stanislav Ochotnicky 2011-05-10 08:47:30 UTC
Thanks for the review, appreciated. As you noticed this was my first emacs package so sorry for those mistakes. Hope everything is OK now. I tested this with ropemacs and it seemed to work including autoload.

* Tue May 10 2011 Stanislav Ochotnicky <sochotnicky> - 0.23-2
- Rename to emacs-pymacs
- Use emacs macros
- Byte-compile elisp source and put sources into subpackage

Spec URL: http://sochotni.fedorapeople.org/packages/emacs-pymacs.spec
SRPM URL: http://sochotni.fedorapeople.org/packages/emacs-pymacs-0.23-2.fc14.src.rpm

Comment 3 Daiki Ueno 2011-05-10 09:30:35 UTC
It looks fine except that:

- Requires: should be "emacs(bin) >= %{_emacs_version}"
- You could use %{name} instead of emacs-%{pkg} everywhere
- The license looks GPLv2+ as far as I checked the files in the source tree

APPROVED.

Comment 4 Stanislav Ochotnicky 2011-05-10 10:39:00 UTC
Thanks for the review, appreciated. I got confused with emacs-filesystem, but in the end I figured it out. Thanks for pointing out the problems.

Final fixes:
Spec URL: http://sochotni.fedorapeople.org/packages/emacs-pymacs.spec
SRPM URL: http://sochotni.fedorapeople.org/packages/emacs-pymacs-0.23-3.fc14.src.rpm

New Package SCM Request
=======================
Package Name: emacs-pymacs
Short Description: Emacs and Python integration framework
Owners: sochotni
Branches: f15
InitialCC:

Comment 5 Jason Tibbitts 2011-05-10 16:04:15 UTC
Git done (by process-git-requests).

Comment 6 Stanislav Ochotnicky 2011-05-11 07:38:59 UTC
Thanks for review and repos. Package built for rawhide and f15:
http://koji.fedoraproject.org/koji/buildinfo?buildID=243445
http://koji.fedoraproject.org/koji/buildinfo?buildID=243444