[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#773992: RFS: xmlrpc-c/1.33.15+svn20141223~2672-1 [ITA]



Hello Paul,

first sorry for my late answer. I was i little bit ooo and then the
31C3.

Many thanks for your review.


Am Montag, den 29.12.2014, 11:13 +0100 schrieb Paul Gevers:
> Control: owner -1 !
> 
> Hi Jörg,
> 
> On 26-12-14 20:48, Jörg Frings-Fürst wrote:
> >   I am looking for a sponsor for my package "xmlrpc-c"
> 
> I am willing to help, review below is incomplete yet though (just
> scanned the changes once).
> 
> >   Alternatively, one can download the package with dget using this command:
> > 
> >     dget -x http://mentors.debian.net/debian/pool/main/x/xmlrpc-c/xmlrpc-c_1.33.15+svn20141223~2672-1.dsc
> 
> I use the git repository. I assume it is the same.
> 
> Ironically, upstream just (15 hours ago) seems to have released 1.33.15
> as tar ball.

I have seen it too. But the download diretory is "Xmlrpc-c Super
Stable/". I thinks its only a new Super Stable release. 


> 
> >   * New debian/patches/200-test_port.diff:
> >     - Change port for testing from 8080 to 7890 (Closes: #722503).
> 
> I am missing some background, either in the bug or in the patch file.
> Why do you think hard coding 7890 is any better than 8080? And why do
> you consider this "Forwarded: not-needed"? Michael suggested to try
> multiple ports instead of relying on one port and I think upstream
> should be interested in such a patch.

My first opinion was to change nothing. On debian the builds was running
in a clean system without network. So there are no changes required.

On a build on a normal system port 8080 often be used by proxies or http
servers. So I switched the port for the build tests to 7890.

I think that the using of multiple ports in this case the coast are to
high.

In my opinion is the best way to set the port for testing at build time
as configure variables (--testport=7890  or so). But this must be
discussed with upstream. So have set "Forwarded: not-needed". 

> 
> >   * New debian/patches/005-xmlrpc_example.diff:
> >     - Backport from upstream release 1.34.0 (Closes: #524550).
> 
> If you still want me to upload your current package, could you add a
> source URL to the DEP5 header? The bts seems to say it should be here:
> http://sourceforge.net/p/xmlrpc-c/code/2491 but with that commit I don't
> see any content there. At least mention the proper revision in the
> headers. Also, the patch seems to fix more than just the bug in the bts.
> Please document what it is supposed to fix.
> 

I have rewriten the patch, so that only the wrong example is removed.


> >   * New missing debian/xmlrpc-c-config.man and debian/xmlrpc.man.
> 
> You created these files with help2man. I prefer it when you do this at
> build time, so that the man page stays up-to-date. I think you can tweak
> the settings of help2man to not add the date and your name.
> 
Yes the manfiles was basically made with help2man, but also massive
edited. Therefore I don't build them at build time.

The author line was requested for the package mwc by Eriberto Mota. I
have added the missing author tag.
 
> Other (random) remarks:
> 
> Please target experimental during the freeze, unless you are fixing
> something that needs to go into jessie.
> 
> Just wondering (haven't check yet), but you only add symbols files for
> amd64 and i386. Is this working correctly with the other archs? Or are
> they going to be more strict as a result?
> 
My error. I have differences between amd64 and i386. So I have renamed
the symbols file for libxmlrpc-c++8 to *.amd86 and *.i386.

I have renamed libxmlrpc-c++8.symbols.amd64 to libxmlrpc-c++8.symbols.


> Please separate your commits to git to ease review and understanding.
> 

ok

> I don't understand why you created a "new upstream release". As far as I
> see it the only change is the version number and the distclean target in
> the Makefile. No further changes in the files that you decided to keep
> around.
> 

Are the upstream change from 1.33.14 to 1.33.15 not enough?


A new version is uploaded to mentors.



> Paul
> 

CU
Jörg

-- 
New:
GPG Fingerprint: 63E0 075F C8D4 3ABB 35AB  30EE 09F8 9F3C 8CA1 D25D
GPG key (long) : 09F89F3C8CA1D25D
GPG Key        : 8CA1D25D
CAcert Key S/N : 0E:D4:56

Old (will be revoked after 2014-12-31):
pgp Fingerprint: 7D13 3C60 0A10 DBE1 51F8  EBCB 422B 44B0 BE58 1B6E
pgp Key: BE581B6E

Jörg Frings-Fürst
D-54526 Niederkail

Threema: SYR8SJXB

IRC: j_f-f@freenode.net
     j_f-f@oftc.net




Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: