On Mon, Nov 25, 2013 at 11:22 PM, Zygmunt Krynicki < zygmunt.kryni...@canonical.com> wrote:
> Hi. > > Thanks for the quick review! > > > On Mon, Nov 25, 2013 at 9:24 PM, Piotr Ożarowski <pi...@debian.org> wrote: > >> > [1] https://github.com/zyga/debian.plainbox >> >> * -doc package is empty, >> > > Odd, it wasn't when I was building it, I must have broken something before > the final packaging commit. > Docs were being added to the plainbox package by mistake, this is now corrected. > > >> * debian/copyright is not complete, there are Python Software >> Foundation, Aaron Iles, Michael Foord and Linaro Limited files in >> plainbox/vendor dir, >> > > All of those will be removed. This is in the debian/TODO.txt, basically > none of the vendor packages are needed in recent enough Debian and Ubuntu > releases. > > >> * about plainbox.vendor - please use packaged versions of this modules >> and drop them from python3-plainbox (at least mock and funcsigs are >> not needed), >> > > As above. > No change so far > > >> * XlsxWriter is not packaged for Debian and you probably need it in >> Recommends or Suggests (if the later one, we can upload it without >> waiting for python3-xlsxwriter package), >> > > Good point, thanks. I'll do that. > > Done * debian/changelog: distribution is wrong, you want to upload to >> unstable, right? >> > > This one was for Ubuntu 14.04, I'll make a branch for unstable (I didn't > expect to get this review that fast :-) > > Corrected. The branch 'sid' is now the master branch and the 'trusty' branch is a derivative with minimum changes. > * "Priority: extra" - please use "optional" by default (unless there's a >> reason to use different priority); FTP masters will later move it to >> "extra" for unknown reason (I suspect they just don't like me and move >> all packages I upload/sponsor to "extra" ;), but IMO the right >> priority is optional in this case, >> > > Noted, thanks. > > Corrected > * add DPMT to Uploaders, >> > > d...@debian.org? > I figured this out and corrected > > >> * Standards-Version 3.9.5 is already out, please check >> /usr/share/doc/debian-policy/upgrading-checklist.txt.gz, >> > > Will do. > > Updated without any changes. > * "plainbox is a Simple replacement for CheckBox" looks weird to me, >> it looks better with lower case "s" IMO (see dev. ref. §6.2.2), >> > > I was reading that and I think that description is no longer accurate. I > will write a better one, that describes the purpose of the project as it > stands now. > > Rewrote descriptions entirely > * there's "XS-Testsuite: autopkgtest" but I don't see any tests in the >> package, >> > > Disabled because of the bug preventing unit tests to run. This will be > fixed in beta2 on Wednesday. > > I've removed XS-Testsuite until this can be re-enabled > * plainbox manpage is missing, >> > > Is it mandatory to have one? I recall reading about not symlinking to > undocumented.7. Plainbox does not currently have a man page but instead has > built-in --help. I guess I'm asking about the minimum required man page > that could be worked upon later. > No change yet > > >> * I'd use s/b/~/ instead of s/b/~beta/ in debian/watch (to keep it as >> close to upstream as possible), and please escape last dot in this >> file. >> >> Noted, will fix this. > Fixed > > >> Fix those and inject to our SVN repo (that's right, we don't use GIT... >> yet) >> > > How should I inject that into SVN? Will I get commit access to the > collaborative repository? > > Before I sort that out I've updated the repository on github. Please have a look and tell me if I missed anything. Best regards ZK
