Code review comment for lp:~clint-fewbar/review-new-branches/ceph-new-pkg

> * debian/changelog
>
> The file should be trimmed to only have one entry mentioning the
> initial packaging for Ubuntu and refering the need-packaging bug in LP
> (bug 607730):
>
> ceph (0.21-0ubuntu1) maverick; urgency=low
>
> * Initial release based on upstream package (LP: #607730).
>
> * debian/control
>
> The maintainer field should be updated per
> https://wiki.ubuntu.com/DebianMaintainerField.

These are packaging fixes on the ubuntu side of things I take it.

> * debian/copyright
>
> I would recommend to format the content of this file according to
> the 'Machine-readable debian/copyright' DEP - see
> http://dep.debian.net/deps/dep5/.
>
> The Files: * section has an empty line on line 14.
>
> src/include/ceph_hash.cc: should also be mentioned as it's copyrighted
> under Public Domain.
>
> src/os/btrfs_ioctl.h: is under GPLv2.
>
> Running 'licensecheck -r .' in the top source directory will output a
> good estimate of the license and copyright for each file.

Fixed.

> * debian/rules: I'd recommend to properly support autotools in the build
> system. See /usr/share/doc/autotools-dev/README.Debian.gz available
> from the autotools-dev package.

I'll do this for the next release.

> * Log files should be properly handled as outlined in the Debian Policy
> Section 10.8 [1]
>
> [1]: http://www.debian.org/doc/debian-policy/ch-files.html#s10.8
>
> Log directory should be deleted on package purge.

Added a ceph.postrm that will rm -rf /var/log/ceph on purge.

> * Configuration files in /etc/ seem to be sample files given their name
> in ceph.install:
>
> etc/ceph/sample.ceph.conf
> etc/ceph/sample.fetch_config
>
> Are these files providing a default working configuration? If so their
> name should not start with sample. If not they should be shipped under
> /usr/share/doc/ceph/ and a default working configuration should be
> shipped as part of the package.

Moved to /usr/share/doc/ceph.

> * AFAICT all commands to create a filesystem are located in /sbin/.
> mkcephfs should probably be there as well instead of
> usr/sbin/.

Moved.

> * debian/rados.install is not needed as there isn't any rados package defined
> in debian/control.

Removed.

> * Upon installation of the ceph package it seems that there isn't a
> default working configuration. Is it possible to provide a default
> working configuration?

There's no way to make a useful default working configuration on a single node (it's a _distributed_ storage system), so /etc/ceph is left empty.

These changes are at git://ceph.newdream.net/git/ceph.git in the 'testing' branch. If they look ok I can make a v0.21.1.