control: tag -1 +moreinfo
control: owner -1 !
Dear Nicholas,
On Sat, Nov 12, 2016 at 10:18:54PM -0500, Nicholas D Steeves wrote:
> I am looking for review and a sponsor for my package "src:muse-el".
> I've CC'd Sean Whitton who is willing to help me with elpa, LISP, and
> any emacs-on-Debian integration issues.
In the future, when submitting a new bug, please use X-Debbugs-CC: rather
than CC: so that the bug gets assigned a number before reaching my
inbox.
I've split my review into two sections: things that I would consider
must-fixes before an upload to Debian, and suggested improvements. The
latter aren't strictly necessary, but they would help demonstrate to a
potential sponsor that you are committed to maintaining this package in
Debian.
Must-fixes
==========
1. Your changelog could do with some work.
- you should close the ITA bug
- "Elpafy!" is not informative; please rewrite
- the change to use 3.0 (quilt) and debhelper compat 9 should be
separate entries
- "Include images used by the QuickStart.muse example" -- what does
'include' mean? Are you installing them? Be specific.
- "Fix misattributed source" -- which?
- "Update format." -- from what to what?
- "add override" -- you could say specifically which overrides you added
- "Install docs and examples the new dh_way." -- it's not really new,
so I would say "Install docs and examples with dh_install* tools."
- "Thank you Kevin Ryde ..." -- it's good to thank people, but I find it
too informal to talk about your personal history of learning the
command ;) How about just "Thank you to Kevin Ryde for suggesting
this change." (and thanks, I didn't know about see(1) either :))
- "texi stuff" -- again, informal and not informative. How about just
s/stuff/documentation/?
> Is debian/changelog too verbose and could it be condensed?
I don't find it to be too verbose.
2. Use spaces, not tabs, to wrap lines in debian/NEWS.
3. README.Debian should have a single timestamp. Remove the previous
maintainer's timestamp, add subheadings for the parts added by each of
you, and put your timestamp at the bottom.
4. The maintainer should be the pkg-emacsen team, with you as an
uploader. See other team packages.
5. There are a lot of Lintian warnings. Please make the package
Lintian-clean.
6. The package fails testing with adequate:
2m16.3s ERROR: FAIL: Inadequate results from running adequate!
elpa-muse: py-file-not-bytecompiled /usr/share/emacs/site-lisp/elpa-src/muse-3.20/getstamps.py
elpa-muse: py-file-not-bytecompiled /usr/share/emacs/site-lisp/elpa-src/muse-3.20/hardcodedates.py
elpa-muse: py-file-not-bytecompiled /usr/share/emacs/site-lisp/elpa-src/muse-3.20/metadate.py
7. These .py files should probably not be installed into
/usr/share/emacs -- or is it impossible to use them outside of Muse?
Have you considered installing them according to the python packaging
policy? Can Muse be patched to use them when installed that way?
> Are the generated packages in the correct sections for an elpafied
> package?
Yup.
> Is removing hrefs to favicon.ico (Bug: #775885) sufficient?
> Distributing a favicon.ico seems unnecessary and honestly, I'm
> against it. In Canada, because the author retains all distribution
> rights in the absence of a license or declared copyright, even
> something as trivial as a favicon.ico can be unredistributable. I
> couldn't find the license on Michael Olson's website
> <mwolson.org>, so I decided it was best not to copy it.
Yes, I think just removing the href is fine.
> Do my patches look ok?
8. They need patch headers, preferably conforming to DEP-3, explaining
what each of them is for. Try to include a Forwarded: header, in
particular.
> Should elpafied packages continue to be arch-independent?
Yup.
Suggestions
===========
1. Consider using dh compat 10.
2. Consider adding Enhances: emacs25.
3. Consider priority: optional, not extra.
4. Please use secure URIs in Vcs-* fields.
5. Please add doc-base registration.
I haven't tried installing and running the package yet, but the above
should be enough to be going on with.
--
Sean Whitton
Attachment:
signature.asc
Description: PGP signature