[Pkg-sql-ledger-discussion] Review of ledgersmb 1.3.10-1

[Robert James Clay]

On Thu, 2012-01-19 at 14:48 -0500, Raphael Hertzog wrote:

> Then I looked at debian/patches/ and the number of patches is a bit
> ridiculous. IMO you should have merged all the similar patches together.

   I'll admit that trying to maintain that many changes to that many
different files all in one patch, or even a small set of patches (there
were only a few different types of issues...), did not really appeal to
me.  There is also that I'm still learning quilt and don't know how easy
it would be to maintain them that way.


> I don't see the upstream developers picking your patches one by one... IMO
> if they want to merge fixes for those POD issues, they want all the fixes
> at the same time.

  One thing is that some of the files involved are legacy files from
before the fork from sql-ledger, which they may not be in much of a
hurry to work on because they want to replace them in any case. I'll
check further with upstream, though.


> Same goes for the shebang patches.

   Those have been resolved upstream (and the ticket closed) and so will
be removed from the packaging for whichever version that shows up in.
(likely v1.3.11...)


> Another small detail: debian/patches/15_httpdconf.patch provides
> a "ledgersmb-httpd.conf.rej" which it shouldn't.

   I'll fix that.


> Another weird thing to investigate: debian/patches/05_confdir.patch
> doesn't modify the $templates variable yet it probably should since
> you install templates in /etc/ledgersmb/templates and not in
> /usr/share/ledgersmb/.

   You're right and that I'll need to investigate it; that patch has
been that way since before I become involved and I'll need to check into
how the template files have and are now being handled.






Jame