On Tue, Jun 16, 2015 at 08:11:45PM +0530, Balasankar C wrote: > Hi Cedric,
> Thanks for the detailed review. I have some doubts though. > On ചൊവ്വ 16 ജൂണ് 2015 12:06 വൈകു, Cédric Boutillier wrote: > > > > A few comments: - you may want to remove the LOAD_PATH modification > > in bin/rotp > Done. This is done because, rotp will be already accessble via > rubygems-integration. Right? Not really rubygems-integration. The path where the lib is installed is in the LOAD_PATH so the interpreter finds it automatically. > > - you may also want to regenerate the doc with yard > I couldn't find any documentation regarding how to do that. Are there > any documentation or reference packages which use this? Just call the `yardoc` command just before dh_installdocs in the override_dh_installdocs target in debian/rules. If you do that, you will overwrite files in doc. You may want to use another path to install the new doc, like "yardoc -o yardoc", and copy the files from yardoc/ to usr/share/doc/ruby-rotp instead of those from doc/. > > - in the man page, the name rotp should be in bold in the NAME and > > SYNOPSIS sections, and SECRET and COUNTER should be italic. > I made it bold in SYNOPSIS. But, I haven't seen it bold in the NAME > section of common programs. gem2deb, for example. Also, did the italics. You're right that no bold is needed in the NAME section. > > - you need libjs-jquery as a dependency (to ensure that it will be > > installed with the package, to be able to use the docs), and not as > > a build-dependency.q > Ah, my bad. I missed that somehow. > > - you repackaged the source to remove a file. You should add a > > comment to debian/copyright to mention which files are removed and > > why. The method you use to remove the file is a gbp filter. Another > > possibility (maybe even recommended) is the use of Files-Excluded: > > paragraph in debian/copyright. See uscan(1). It has the advantage > > of centralizing the removal (instructions/documentation) in one > > place. > It seems lintian gives me error on that (most probably because it is > not added to copyright format 1.0). So, I added the repack detail as > follows. Please see if that is enough With a Files-Excluded: command, the debian/watch file needs a uversionmangle option, which doesn't make lintian happy. I don't know if that's what you're talking about. > Format: > http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ > Upstream-Name: rotp > Source: https://github.com/mdp/rotp > The upstream source contained jquery.js file without source. > It is > repacked to > remove that file Should be ok. Cédric
signature.asc
Description: Digital signature