[Debian-olpc-devel] 'rainbow' uploaded to mentors.debian.net

[Jonas Smedegaard]

On Sat, Aug 29, 2009 at 12:41:44PM -0400, Luke Faraone wrote:
>On Sat, Aug 29, 2009 at 05:52, Jonas Smedegaard <dr at jones.dk> wrote:
>> I suggest patching Makefile to comment out python calls, and then 
>> include python-distutils.mk and let that cdbs snippet take care of 
>> proper Python handling (that snippet needs hints in debian/control 
>> and DEB_PYTHON_SYSTEM set - tell me if you need guidance about 
>> those).
>>
>
>Yep, that fixed it.

Good.


>I've split off the package into two separate binary packages, added 
>manpages, and fixed the /usr/local problems.
>
>Can you take a look at the most recent push to see if there are any 
>additional problems?

Your last commit contains (at least) 4 independent changes.  Please 
commit each kind of change separately - that makes it much easier for 
others to follow the flow of code changes, and also easier to revert 
something if needed.

Why do you patch to install into /usr/sbin rather than /usr/bin?  Aren't 
those binaries supposed to be executed by ordinary users?

I notice that you include ${cdbs:Recommends} and similar in 
debian/control, but still include explicit dependencies as well.  The 
intend of those variables is to move all package dependency handling to 
debian/rules.  See debian/rules of e.g. sugar for an example.

Why provide a virtul package olpc-security? Do you plan to package other 
alternative ABI-compatible security mechanisms?

Similar with nss-rainbow - why provide such virtual package?

Short descriptions should avoid leading "a" or "the", I believe.

Long descriptions says "on the OLPC" which is incorrect, I believe: OLPC 
is an organization - I suspect that you mean "on the XO laptops by OLPC" 
or something similar.

Where is the "upstream" branch holding the virgin upstrema code?  Did 
you not use git-import-orig to bootstrap the packaging?  "pristine-tar" 
branch is missing too.  Please use both --pristine-tar and --sign-tags 
options to inject upstream tarballs.

Here's how I usually start a new packaging:

wget http/example.org/pkgname-1.2.tgz
mv pkgname-1.2.tgz pkgname_1.2.orig.tar.gz
mkdir pkgname
cd pkgname
git init
git-import-orig --sign-tags --pristine-tar ../pkgname_1.2.orig.tar.gz

You can either start from scratch again (and please do apply each change 
as a separate commit to help tracking what is what), or I can try merge 
a virgin proper bootstrap with your branch.  Tell me how you want us to 
deal with it.

Long decription of the NSS module includes its licensing - I find that 
irrelevant there.

I suggest you look at the short and long descriptions of other NSS 
modules and borrow the most common language and writing style from 
those.

Why do you include the version number of the NSS module?  I suspect only 
shared libraries should include a trailing major version, and that NSS 
modules are not normal shared libraries.  Again, compare with other NSS 
modules for how they do things (without duplicating blindly - e.g. we 
use CDBS while some of them might use incompatible debhelper 7 "dh").

Why do you install /var - that looks odd to me.

Are you sure you should install /usr/lib/python*? - I suspect that is 
taken care of by the python-distutils.mk snippet, and installing 
directly actually violates Python policy!

Why do you set DEB_MAKE_MAKEFILE?

Why set DEB_PYTHON_MODULE_PACKAGES - I believe that should be 
autodetected if using proper package names.  Even if not, I still 
suspect that you should rename the package to include a leading 
"python-" to not violate Python Policy.

I dislike you claiming authorship of the manpages, when in fact 
autogenerated using help2man.  I recommend stripping that sentence from 
debian/help2man.

I suggest renaming debian/help2man to either debian/help2man.include or 
debian/help2man.addon. to clarify its purpose.

I suggest renaming *.md to *.mdwn (the extension used by e.g. Ikiwiki 
for Markdown files - I am not entirely sure that there is consensus 
about that extension, though).

Did you write those Markdown files from scratch yourself, or did you 
bootstrap from data pulled from e.g. some web pages?  If so, it might 
make sense to setup a target in debian/rules to regenerate using newer 
upstream web sources.  Such target should not be used in normal builds 
(that's illegal!) but can be used by us to check once in a while if 
there are updated documentation.

I suggest dropping the patch to clean egg file, and instead do that in 
clean:: of debian/rules: We want to limit patching to the least 
possible.

You should consider use the new proposed patch hints (even if I haven't 
done so yet on most of the packages I maintain): 
http://dep.debian.net/deps/dep3/

Why not use the patch naming policy that I suggested in previous mail?



Kind regards,

  - Jonas

-- 
* Jonas Smedegaard - idealist & Internet-arkitekt
* Tlf.: +45 40843136  Website: http://dr.jones.dk/

  [x] quote me freely  [ ] ask before reusing  [ ] keep private
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/debian-olpc-devel/attachments/20090830/46c8902f/attachment.pgp>