Il 29/12/2016 18:17, Mattia Rizzolo ha scritto:
> On Tue, Nov 29, 2016 at 08:20:23AM +0100, Paolo Greppi wrote:
>> Hi,
> 
> Hi!
> 
> FYI, I found your RFS only thanks to the /topic in #debian-python.
> Unless you're very lucky most RFSes sent to random mailing lists have a
> tendency to get lost/ignored; that's why I suggest you always file a RFS
> bug and X-Debbugs-CC the relevant team, unless you know that team is
> going to react (like pkg-js recently).

OK thanks for the suggestion

>> I packaged python-patch as per this ITP:
>> https://bugs.debian.org/845482, this is the repo:
>> https://anonscm.debian.org/cgit/python-modules/packages/python-patch.git
>>
>> Please someone more experienced than me review it and if it's OK sponsor
>> its upload.
> 
> I fixed the file name in the pristine-tar branch (otherwise `origtargz`
> ignored it..).

I rebuilt the repo from scratch anyway...

>> Please note that since the pypi tarball has no tests, whereas the github
>> tarball has no setup, I choose the latter and added the setup.py with a
>> git-dpm/quilt patch. I hope this is correct.

I had incorrectly added setup.py to master.
In the new repo I removed the setup.py file from the master branch, it is now 
really added by the patch.
This caused some trouble with the auto_clean target which I think I addressed.

> Yep, that's fine.  Please ask upstream to syncronize both, and have
> github ship the setup.py, and the tarball the release.

https://github.com/techtonik/python-patch/issues/51

> more changes I ask you:
> * d/changelog:
>   + please kill the second changelog line; first uploads should only
>     come with a "first upload" line
>   + finalize it (dch -r)

done

> * d/control:
>   + please wrap-and-sort that list of build-deps
>   + why are you commenting out the Testsuite field?
>   + Vcs-* are pointing to a repo that's not DPMT's, that's wrong
>     (furthermore that URL first requires auth, and it gave me a 404, so
>     I think it's a private repo)

done; I have uncommented the testsuite (not sure why it was commented)

> * d/compat:
>   + please bump to 10 (d/control already have the >= 10, so I guess you
>     just forgot to push this one too)

done

> * d/rules:
>   + please repspect DEB_BUILD_OPTIONS=nocheck

should be OK (see below)

>   + please use the method provided by pybuild to properly run the tests
>     against all supported python versions, against what you just
>     "built"; I think that one runs only one python version (2.7)
>     against the original sources.

I have added a test_suite config to setup.py (and moved from distutil to 
setuptools); this testsuite is then picked up by pybuild, and AFAICT it honors 
the nocheck flag automatically

>   + you're overriding dh_auto_install when you only want to append
>     --install-script to the command invoked.  Please use
>     PYBUILD_INSTALL_ARGS=--install-scripts=... instead.

OK

> * d/copyright:
>   + why are you licensing debian/ under a different license?

because I prefer GPL for my contributions to Debian

>   + personally I find a lot more readable to have all the file paragraph
>     at the top, and all stand alone licenses at the bottom

OK

>   + other/pack.py is under another license

OK added a public domain license for that

> * I: python-patch: new-package-should-not-package-python2-module python-patch

OK I stripped all python2 stuff

>   + right, I was about to forget about this...
> * I: python-patch source: binary-control-field-duplicates-source field 
> "section" in package python-patch

OK (solved by the previous one BTW)

Thanks for all ! It should be now ready to go.

Paolo

Reply via email to