Hi Lance, when re-reading Nilesh's mail I agree with all his points. I've fixed several of them when reviewing.
Am Thu, Feb 03, 2022 at 07:51:33PM +0530 schrieb Nilesh Patra: > On Thu, Feb 03, 2022 at 01:50:39PM +0000, Lance Lin wrote: > > Hi Nilesh, > > > > On Wednesday, February 2nd, 2022 at 9:33 PM, Nilesh Patra > > <[email protected]> wrote: > > Ok, I've tested the package and the only warning is an improbable bug > > number. > > However, it is the correct bug number. > > I do not see that problem; since you see a probable bug number problem, most > likely > you are using an older version of lintian (before we had our on-million'th > bug report) > Please upgrade +1 > > It is hosted on the debian med namespace [1]. > > Please review the work, but I think it should be complete. > > Okay, here's the review: > > 1. Naming: Since this is a header-only lib, IMO it can/should be named to > libtoml11 instead I agree. Please rename. > 2. Branch structure is wrong > > + We follow dep-14 protocol which means there are three branches, master; > upstream and pristine-tar. Please skim-through our > policy[2] and dep-14 docs[3] should you find it useful I've created those branches and like to stress that reading the suggested links is a good advise. > + Looks like you are not using gbp, using it can save you from these > pitfalls. +1 > @Andreas, can you link here to a recent packaging tutorial of yours, using > gbp? Lance confirmed he has seen at least parts of my DebConf workshop. > 3. Remove debian/debhelper-build-stamp; that's the default debhelper > behaviour, you don't need this Done. > 4. Remove debian/files; did you commit this file off after doing a > dpkg-buildpackage? Are you not building in a clean chroot? Done. > 5. d/patches is empty, so purge this Done. > 6. d/rules: > > + "export DEB_CFLAGS_MAINT_APPEND = -Wall -pedantic" is un-needed, that'd > get these by default > + "export INSTALL_PREFIX = /usr/" -- I do not see it when I grep the code; > so I think this is un-needed. > Debhelper will take care of the prefix on its own, provided the > upstream build does not override it Fully ACK - please remove. > 7. d/control: > > + Maintainer field should be 'Debian Med packaging team' along with our > alioth list email; take a look at > any package on our namespace's salsa > + You should be mentioned in uploaders Done. > + Bump Standards-Version to 4.6.0 > + Add Rules-Requires-Root field Both done > + Binary package name is wrong, it should be libtoml11-dev; check library > style packaging guide[4] ACK. Please rename > + Description seems a bit weird, you might want to reformat it ACK. Also the wording is a bit monotone to read but we could possibly live with this. > 8. No d/upstream/metadata: github-debian-upstream script from pkg-js-tools > can help create one Done by using lintian-brush (called by routine-update) > 9. No autopkgtests: Since you are familiar with these (you added it to a > couple of packages) you might want to have > it in here too. ACK. > Welcome, hope this helps. I hope Andreas can steer this discussion after this > point; less time in the week :-) Lance, it would be nice if you would work down such a list of todos as created by Nilesh and ask for a second review afterwards. Please come back here once you fixed the remaining items. Please note that I bumped the upstream version to latest upstream when I did the changes (partly by calling routine-update). Meanwhile I learned that the package uncalled is featuring a code copy of toml11 so I see a good reason to maintain it in Debian Med team. Kind regards Andreas. > > [1] https://salsa.debian.org/med-team/toml11 > [2]: > https://med-team.pages.debian.net/policy/#to-create-a-new-local-git-repository > [3]: https://dep-team.pages.debian.net/deps/dep14/ > [4]: https://www.netfort.gr.jp/~dancer/column/libpkg-guide/libpkg-guide.html > > Regards, > Nilesh -- http://fam-tille.de
