Dear Sean, On Sun, Nov 20, 2016 at 5:15 AM, Sean Whitton <spwhit...@spwhitton.name> wrote: > Thank you for your work to bring this new package to Debian! As I said, > I can't sponsor the upload, but I hope this more detailed review is > useful to you.
Of course, your precisive review is very useful for me. Thanks. > I've split it 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. OK. > Must fixes > ========== > > 1. The line "Only support emacs and xemacs" doesn't make sense (what > else would you be supporting?). What were you trying to say? Fixed. It's my simple mistake. I should say "Support emacs and xemacs." > 2. There is a Lintian error: > > E: verilog-mode: info-document-missing-dir-section > usr/share/info/verilog.info.gz You are right. I miss it out. It's fixed by debian/patches/fix-dircategory.patch. The patch is pull-requested to upstream: https://github.com/veripool/verilog-mode/pull/13 > 3. Some files are not GPL-3+. For example, > tests/auto_delete_whitespace.v. Please check every file's copyright > status and detail in d/copyright. The debian/copyright is updated. > 4. The README is useless to an end user who has already installed the > package, so you shouldn't be installing it -- it could be confusing. Fixed. > 5. You've missed some steps of the Emacs policy.[1] For example, you > are missing a emacsen compat level. Please check the policy carefully. > > [1] https://www.debian.org/doc/packaging-manuals/debian-emacs-policy I added "verilog-mode.emacsen-compat" file. And I think that there are no other violation. > Suggestions > =========== > > 1. It would be best to build-depend on emacs25, not emacs24. emacs24 > might be removed from stretch. Fixed. > 2. At debhelper compat 10, you can probably delete > debian/verilog-mode.dirs. You are right. It's removed from git repo. > 3. You are generating ChangeLog.txt but not installing it. You can use > dh_installchangelogs(1). I think it's not useful. ``` $ make ChangeLog.txt ./make_log.pl Unknown host nasuno. $ cat ChangeLog.txt $ make ChangeLog perl makechangelog --git verilog-mode.el > ChangeLog $ cat ChangeLog -*- Change-Log -*- <Put one line comment with trailing period here. This must be first line of commit message.> * verilog-mode.el: ====================================================================== ``` > 4. How about installing verilog-lex.el as an example? See > dh_installexamples(1). Or possibly somewhere else. Sorry. I can't understand why it becomes as example, because I'm not a expert for both Emacs and Verilog. Best regards, -- Kiwamu Okabe at METASEPI DESIGN