Bug 379751
| Summary: | Review Request: emacs-common-ess - Emacs Speaks Statistics add-on package for Emacs | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Alex Lancaster <alex> |
| Component: | Package Review | Assignee: | Jason Tibbitts <j> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, j, notting, petersen |
| Target Milestone: | --- | Flags: | j:
fedora-review+
kevin: 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: | 2007-11-20 02:15:55 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
Alex Lancaster
2007-11-13 11:59:22 UTC
I still think the prefix in the Emacs packaging guidelines should be changed from emacs-common to a single word like emacs. The packages do end up with "a single word like emacs". Specifically, "emacs".
The data that's shared between the emacs and xemacs packages is called
emacs-common. I don't see the naming of the source package to be a real issue
since users don't see it, but if the guidelines had be presented suggesting that
the SRPM be named emacs-foo regardless of which emacs variant the package
supports then I wouldn't have objected although I suppose that would look odd in
the case of an xemacs-only package.
Seems to build OK for me, although I saw complaints about not being to find any
version of R installed. Will the built packages properly support R if it's not
in the buildroot?
Not that it matters, but why not use %{pkgname} in the Summary, too?
The install-info dependencies aren't quite right. If you use something in
%post, you need Requires(post).
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets should have a template
for installing texinfo files.
rpmlint says:
emacs-common-ess.noarch: W: file-not-utf8
/usr/share/doc/emacs-common-ess-5.3.6/ChangeLog.lisp
emacs-common-ess.noarch: W: file-not-utf8
/usr/share/doc/emacs-common-ess-5.3.6/doc/TODO
emacs-common-ess.noarch: W: file-not-utf8
/usr/share/doc/emacs-common-ess-5.3.6/ChangeLog
These should be passed through iconv.
emacs-ess.noarch: W: no-documentation
emacs-ess-el.noarch: W: no-documentation
xemacs-ess.noarch: W: no-documentation
xemacs-ess-el.noarch: W: no-documentation
I don't think these are problematic.
emacs-ess.noarch: E: non-executable-script
/usr/share/emacs/site-lisp/ess/etc/backbugs.sparc 0644
emacs-ess.noarch: E: non-executable-script
/usr/share/emacs/site-lisp/ess/etc/ess-sas-sh-command 0644
emacs-ess.noarch: E: non-executable-script
/usr/share/emacs/site-lisp/ess/etc/backbug5 0644
emacs-ess.noarch: E: non-executable-script
/usr/share/emacs/site-lisp/ess/etc/backbug5.sparc 0644
emacs-ess.noarch: E: script-without-shebang
/usr/share/emacs/site-lisp/ess/etc/BACKBUGS.BAT
I'm not really sure what the point of these files (and the rest of the ess/etc
directory) is. Certainly the BAT files have no real place on Linux. Are any of
them actually relevant? What will call them?
(In reply to comment #2) > Seems to build OK for me, although I saw complaints about not being to find any > version of R installed. Will the built packages properly support R if it's not > in the buildroot? Builds OK in koji without R http://koji.fedoraproject.org/koji/taskinfo?taskID=246902 I then also downloaded the builds and tested them locally and editing R worked as did the interactive R mode. > Not that it matters, but why not use %{pkgname} in the Summary, too? Done. > The install-info dependencies aren't quite right. If you use something in > %post, you need Requires(post). > http://fedoraproject.org/wiki/Packaging/ScriptletSnippets should have a template > for installing texinfo files. Fixed. > rpmlint says: > > emacs-common-ess.noarch: W: file-not-utf8 > /usr/share/doc/emacs-common-ess-5.3.6/ChangeLog.lisp > emacs-common-ess.noarch: W: file-not-utf8 > /usr/share/doc/emacs-common-ess-5.3.6/doc/TODO > emacs-common-ess.noarch: W: file-not-utf8 > /usr/share/doc/emacs-common-ess-5.3.6/ChangeLog > These should be passed through iconv. Fixed > emacs-ess.noarch: W: no-documentation > emacs-ess-el.noarch: W: no-documentation > xemacs-ess.noarch: W: no-documentation > xemacs-ess-el.noarch: W: no-documentation > I don't think these are problematic. > > emacs-ess.noarch: E: non-executable-script > /usr/share/emacs/site-lisp/ess/etc/backbugs.sparc 0644 > emacs-ess.noarch: E: non-executable-script > /usr/share/emacs/site-lisp/ess/etc/ess-sas-sh-command 0644 > emacs-ess.noarch: E: non-executable-script > /usr/share/emacs/site-lisp/ess/etc/backbug5 0644 > emacs-ess.noarch: E: non-executable-script > /usr/share/emacs/site-lisp/ess/etc/backbug5.sparc 0644 > emacs-ess.noarch: E: script-without-shebang > /usr/share/emacs/site-lisp/ess/etc/BACKBUGS.BAT These should be fixed now. > I'm not really sure what the point of these files (and the rest of the ess/etc > directory) is. Certainly the BAT files have no real place on Linux. Are any of > them actually relevant? What will call them? Some of these files are needed at runtime (they are called out by the Lisp code) such as backbugs. I've removed all the unnecessary files such as the BAT files, and others which shouldn't have been installed in the first place (now in %doc), leaving just the files that the Lisp package appears to actually use. So here are the updated files: * Sun Nov 18 2007 <alexlan> - 5.3.6-2 - Moved all non-code related files in etc/ to documentation directory - Make sure all doc files are UTF-8 - Fix post, preun scriptlets and add Requires for installing info files Spec URL: http://alexlan.fedorapeople.org/reviews/emacs-common-ess.spec SRPM URL: http://alexlan.fedorapeople.org/reviews/emacs-common-ess-5.3.6-2.fc7.src.rpm rpmlint is down to just the four no-documentation complaints, which are OK. Everything looks good to me. * source files match upstream: 9531c53e534550924680862e10c04eadd9c049c66a1fc0b9df940c1f966c7a12 ess-5.3.6.tgz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * Emacs packaging guidelines look to be followed properly. * summary is OK. * description is OK. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly * rpmlint has acceptable complaints * final provides and requires are sane: emacs-common-ess-5.3.6-2.fc9.noarch.rpm emacs-common-ess = 5.3.6-2.fc9 = /bin/sh /sbin/install-info emacs-ess-5.3.6-2.fc9.noarch.rpm emacs-ess = 5.3.6-2.fc9 = /bin/sh emacs(bin) >= 22.1 emacs-common-ess = 5.3.6-2.fc9 emacs-ess-el-5.3.6-2.fc9.noarch.rpm emacs-ess-el = 5.3.6-2.fc9 = emacs-ess = 5.3.6-2.fc9 xemacs-ess-5.3.6-2.fc9.noarch.rpm xemacs-ess = 5.3.6-2.fc9 = /bin/sh emacs-common-ess = 5.3.6-2.fc9 xemacs(bin) >= 21.5.28 xemacs-ess-el-5.3.6-2.fc9.noarch.rpm xemacs-ess-el = 5.3.6-2.fc9 = xemacs-ess = 5.3.6-2.fc9 * %check is not present; no test suite upstream. Frankly I've no idea how to test this so I'll trust the maintainer. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * scriptlets are OK (texinfo index generation) * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. APPROVED New Package CVS Request ======================= Package Name: emacs-common-ess Short Description: Emacs Speaks Statistics add-on package for Emacs Owners: alexlan Branches: F-7 F-8 Cvsextras Commits: yes Actually can you modify that to add Tom Moertel (tmoertel)? New request is: New Package CVS Request ======================= Package Name: emacs-common-ess Short Description: Emacs Speaks Statistics add-on package for Emacs Owners: alexlan, tmoertel Branches: F-7 F-8 Cvsextras Commits: yes (In reply to comment #2) > The packages do end up with "a single word like emacs". Specifically, "emacs". > The data that's shared between the emacs and xemacs packages is called > emacs-common. I don't see the naming of the source package to be a real issue > since users don't see it, Well that is what appears in bugzilla, the buildsys, and other places, so IMHO it is actually pretty important - specially as emacs-common-foo packages start to proliferate. > but if the guidelines had be presented suggesting that > the SRPM be named emacs-foo regardless of which emacs variant the package > supports then I wouldn't have objected although I suppose that would look odd > in the case of an xemacs-only package. I think xemacs only packages should be named xemacs-foo. Anyway looks like we already have one more emacs-common-foo... :-/ (In reply to comment #6) > Actually can you modify that to add Tom Moertel (tmoertel)? New request is: Just noticed by chance that tmoertel still needs a sponsor. (In reply to comment #8) > (In reply to comment #6) > > Actually can you modify that to add Tom Moertel (tmoertel)? New request is: > > Just noticed by chance that tmoertel still needs a sponsor. OK, maybe revert my package request to the original and I can add him as co-maintainer after he's sponsored: New Package CVS Request ======================= Package Name: emacs-common-ess Short Description: Emacs Speaks Statistics add-on package for Emacs Owners: alexlan Branches: F-7 F-8 Cvsextras Commits: yes cvs done. Builds done in devel/rawhide. Pending pushes in F-7, F-8. Closing bug. |