On Thu, Jan 25, 2018 at 11:24 PM, Antoine Beaupre <anar...@debian.org> wrote: > I have found some minor issues in the package that I think should be > fixed. > > 1. the lintian-override is not necessary. binary-without-manpage is > just a warning, not an error, and we can live with it until the > binary moves to the right place. in fact, it provides a good > reminder that this still needs to be done, so do not override it. > > 2. in gbp.conf, "upstream-tree = 0.4.3" is out of place. it shouldn't > be necessary because "upstream-tag = %(version)s" should work.
I have fixed these two issues. The override was already there, in the original package. I have removed it as you suggested. It should be fixed, not suppressed. > Other comments, which are not blockers for an upload - but if you're > going to reroll the package anyways, might as well consider fixing this: > > 3. could you clarify why the override_dh_autoreconf target is > necessary? isn't autoreconf exactly designed for the purpose of > running autogen.sh? >From man page 'dh_autoreconf is responsible for calling autoreconf...'. The autogen.sh contains also another command: intltoolize --force > 4. it is quite strange to see a 10k line diff for a patch release > (0.4.2 -> 0.4.3). I would encourage you to release a new *major* > version (e.g. 0.5.0 or 1.0.0) next time you make such significant > changes. not sure you can fix this now that the tag is public on > github, so maybe just a note for next time... How did you perform diff? I ran following: $ tar -xvf ddccontrol_0.4.3.orig.tar.gz $ tar -xvf ~/Downloads/ddccontrol_0.4.2.orig.tar.gz $ diff ddccontrol-0.4.2 ddccontrol-0.4.3 | wc -l 305 But, there are messages in diff, that some files are not present in latest version: Only in ddccontrol-0.4.2: ABOUT-NLS Only in ddccontrol-0.4.2: aclocal.m4 Only in ddccontrol-0.4.2: ChangeLog Only in ddccontrol-0.4.2: config.guess Only in ddccontrol-0.4.2: config.rpath Only in ddccontrol-0.4.2: config.sub Only in ddccontrol-0.4.2: configure Only in ddccontrol-0.4.2: depcomp Only in ddccontrol-0.4.2: INSTALL Only in ddccontrol-0.4.2: install-sh Only in ddccontrol-0.4.2: intltool-extract.in Only in ddccontrol-0.4.2: intltool-merge.in Only in ddccontrol-0.4.2: intltool-update.in Only in ddccontrol-0.4.2: ltmain.sh Only in ddccontrol-0.4.2: m4 Only in ddccontrol-0.4.2: Makefile.in Only in ddccontrol-0.4.2: missing Only in ddccontrol-0.4.2: mkinstalldirs Only in ddccontrol-0.4.2: README These are mostly generated files, which shouldn't be in original sources. New files are: Only in ddccontrol-0.4.3: autogen.sh Only in ddccontrol-0.4.3: CheckList Only in ddccontrol-0.4.3: commit.sh Only in ddccontrol-0.4.3: .gitignore Only in ddccontrol-0.4.3: README.md Only in ddccontrol-0.4.3: .travis.yml These files aren't code related. And, some of them probably need some cleanup. However, version 1.0.0 will be released once two major issues are done: * public API (headers, D-Bus) will be finished, and properly documented, * issue with root access/privileges requirement is fixed. > 5. as lintian noticed, debian/copyright now has a formal syntax, > specified by DEP5. no big deal, but you could do a refresh on that > file. decopy is a tool that does this well. I just found that there's --pedantic option to lintian. Well, there are multiple notices, which should be fixed. I'll try to fix them all with next releases. > Once issues 1 and 2 are fixed, I'm happy to upload the package. Great. The new upload is now present on mentors: https://mentors.debian.net/package/ddccontrol Kind regards, Miroslav Kravec