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

Re: RFS: ocropus (3nd try)



Hello Jeffrey,

On Tue, Dec 2, 2008 at 22:20, Jeffrey Ratcliffe
<jeffrey.ratcliffe@gmail.com> wrote:
> Apologies for the delay in making the changes.

Ah, no problem: take your time

> 2008/11/10 Sandro Tosi <matrixhasu@gmail.com>:
>> - please explain what OCR is in the long description, so that even
>> un-experienced users may understand what the package does
>
> Done.

another thing about description: ocropus and ocropus-data have the
same description: please clarify what's the purpose of -data package,
just adding a simple paragraph at the bottom saying what the package
contains.

>> - what about include in debian/ocroscript.1 the options listed at
>> http://sites.google.com/site/ocropus/documentation ? consider the
>> situation where a user has no net connection and still wants to use
>> ocroscript. And repeating the long description in the manpage adds no
>> information to the users: please expand it to be usefull and please
>
> Done.

Mh, well I meant to take the text from the site and put it into the
file, but at least a reference might be fine.

>> - please add a debian/watch file, "man uscan" for examples
>
> I don't think this is possible for software hosted by Google

yes it's possible:

version=3
http://code.google.com/p/mpmath/downloads/list
http://mpmath.googlecode.com/files/mpmath-(.*)\.zip

(adapt to your needs).

>> - patches have no documentation about what they do; dpatch added a "#
>> DP: " line where you can explain what the patch does (along with other
>> information like the patch author), do you mind add such information
>> (to be clear about the patch scope)?
>
> I've just discovered that you can put comments # at the top of a quilt
> patch and refresh respects them. Done.

Great thanks!

>> - given that "debian/patches/distclean" added some files to be deleted
>> by upstream distclean target, and "clean: unpatch" this means that the
>> patch is removed before clean target in debian/rules is called, hence
>> invalidating the patch. One solution could be add another target:
>>
>> clean: clean-patched unpatch
>> clean-patched: patch-stamp
>>   <commands for the clean target>
>>
>> - It's usually better depends on patch-stamp (or the exported quilt
>> variable, check man quilt) instead of patch, so please fix build-stamp
>> target
>
> OK. I think I've done what you want.

yep you did, just some other suggestions for the clean target:

- "mv Jamrules.old Jamrules" can be simply "rm Jamrules" (removing the
salvage in configure target): that's because if you change a
"generated" file (like in this case) that's present in the upstream
tarball and you want to avoid the modification to appear in the
diff.gz, then simply remove that file in the clean target.

>> - do you need dh_installexamples and dh_install in binary-arch target ?
>
> Moved to the install target.

no no, please move all dh_install<something> calls back in
binary-arch, what I was suggesting is to remove them, since not needed
at all; but I have to  correct myself: only dh_installexamples is to
be removed :) 'cause you have .install files

>> - License section is indeed the copyright one
>
> Sorry - what is your point here?

have a look at [1] where "upsteam author", "copyright" and "license"
are clearly separated, I expect the same for this package :)

[1] http://packages.debian.org/changelogs/pool/main/c/checkgmail/current/copyright

>> - please add the correct license section, referring to apache-2.0,
>> adding the apache license boilerplate (as visible in
>> ./ocrocmd/version.h)
>
> Done.

mh...

>> - since you claim that your packaging is gplv3, and so are the patches
>> you wrote, are you sure that those patches can be applied to
>> apache-2.0 source code? please check. That's the reason we usually
>> suggest to license the packaging under the same license as upstream
>> code.
>
> Done.

better if you state clearly it's Apache-2.0 and not only Apache-2

>> - you completely missed to add information about "External Software"
>> in debian/copyright
>
> What do you mean by this?

that you have to name the copyright owner of those files in
debian/copyright, but not as the main project holder; take this[2] as
an example

[2] http://packages.debian.org/changelogs/pool/main/a/amule-emc/current/copyright

>> - please check *every* source file: ./ocrocmd/version.h has a
>> different copyright year, and many others (2006-2008 seems to be the
>> right years); ./colib/nbest.h (and with it, many other) has different
>> copyright holder, and you have to list them in the debian/copyright
>> file; ext/lua/lua.h (and all the related files) has different
>> copyright holder and license: please add them too and check the
>> license is compatible with apache-2.0.
>>  I can continue with many other files: please check the whole
>> upstream tarball, *every* files, (I exec, from the root level, "find .
>> -type f -exec less {} \;") and report all the copyright and license
>> information differing from the "main" ones and whether they are
>> compatible each other.
>
> licensecheck -r --copyright * only finds Apache-2 or unknown.

please don't rely only on licensecheck, that's just a helper that does
not avoid you to check *every* file (I want to emphatize it because
that's what ftp-master will do) to check that the license and
copyright holders are the same than the main project, and if not
report this difference in debian/copyright.

You might have a look at [3] as a proposed format d/copyright

[3] http://wiki.debian.org/Proposals/CopyrightFormat

> I've uploaded the package again.

If you have additional question, don't hesitate to ask me, I'm here to
help you out :)

Regards,
-- 
Sandro Tosi (aka morph, Morpheus, matrixhasu)
My website: http://matrixhasu.altervista.org/
Me at Debian: http://wiki.debian.org/SandroTosi


Reply to: