Dear Kiwamu, On Fri, Nov 25, 2016 at 09:41:16PM +0900, Kiwamu Okabe wrote: > On Sun, Nov 20, 2016 at 5:15 AM, Sean Whitton <spwhitton@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. Just a few more things and it will be ready for upload :) After you make changes, you need to run `dch -r` to update the timestamp in d/changelog. > > 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." In that case, I think you should just delete that line altogether. Normally, uploads of new packages have a changelog entry with only a single line, closing the ITP. > > 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 Good work. Please add a DEP-3 patch header, including a Forwarded: field to show that the patch has been forwarded upstream. > > 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. The copyright for Makefile is still not correct. > > 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. Looks good. > > 3. You are generating ChangeLog.txt but not installing it. You can use > > dh_installchangelogs(1). > > I think it's not useful. Agreed. Thank you for confirming. > > 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. It has a function `verilog-lex' which might be useful to someone. Maybe it would be better to install it with the other *.el files, and bytecompile it? I'm not sure what I was thinking when I suggested installing it as an example. -- Sean Whitton
Attachment:
signature.asc
Description: PGP signature