Guidelines for reviewing new packages.

[Emmet Hikory]

On Nov 17, 2007 5:52 AM, Stefan Potyra <sistpoty at ubuntu.com> wrote:
> My fear right now with these guidelines is that these will turn to be the
> ultimate criteria to +1/-1 a source package and people will not check if a
> package is in fact good or bad but rather go through this checklist w.o.
> looking at the bigger picture.

    That's a good point.  My motivations in drafting this was to 1)
make it easier for packagers to pre-review, 2) provide a common set of
tests for packages, as I think we all have slightly different
practices with reviewing.  I certainly don't mean for these to become
the official tests, with reviewers not also checking other things, nor
ignoring the focus on distribution-as-a-whole.

> I'm not too sure if micro-managing individual aspects will lead to better
> source packages in general.

    I'm certain it won't, but hope that it will lead to fewer packages
in REVU that fail to build from source, produce empty binaries, don't
have manuals, etc.  The intended audience is both packagers and
reviewers, in the hopes that packagers will perform some review checks
prior to upload.

> Nonetheless, I've tried to write everything which came to my mind for the
> given guidelines.

    Thank you for the detailed review.  Please find my comments below.
 Based on the conclusion of this thread, I'll update the wiki
(although others are welcome to do so beforehand).

> >     2.  Package must match current Ubuntu (and Debian) packaging policies
>
> Unclear: what is the "packaging policy" (debian-policy?)

    This is an interesting point.  debian-policy often receives
updates after a practice has been supported by all available tools for
some time, and has been adopted by a wide number of packages.  Ubuntu
packaging policies seem scattered on the Ubuntu wiki, and some
additional Debian policies (e.g. python, perl, java, etc.) see
likewise scattered (although often on the Debian wiki).  I'd welcome
any suggestions on how to rephrase this to indicate that the relevant
policies should be reviewed, and the package checked for compliance.

> >     3.  Package should be linda & lintian clean
>
> I'd rather put this to not produce unnecessary linda/lintian warnings (not all
> lintian warnings are equally sane imho, and I'd pretty much like people to
> not put in overrides to hide such warnings).

    I tend to prefer no output, and look at any present overrides when
looking through diff.gz.  My experience with REVU is that nearly every
package can be made clean, and I'd suggest that if there is a check
that is inappropriate, or encourages a behaviour thought incorrect,
that lintian be patched to address that, rather than having packages
behave in two different ways, depending on whether the reviewer found
the warnings of merit.

> >     7.  Package should follow [WWW]
> > http://www.debian.org/doc/manuals/developers-reference/ch-best-pkging-pract
> >ices.en.html
>
> Parts of this is outdated for Debian (Homepage, XS-Vcs), for Ubuntu as well?

    True.  Perhaps this should be dropped?

> >         (If upstream is no more, the packager should consider adopting
> > the upstream package somewhere)
>
> What is wrong with using the Ubuntu archive for this?

    Many REVU packages seem to be of the upload-and-forget variety,
and the edges of universe don't always receive strong maintenance.  My
fear is that we'll have more unmaintained and dead packages on the
fringes.  If an upstream project exists (LP may be good for this), a
community of users can be built (perhaps not all using Ubuntu) to keep
the software in shape.  If someone finds upstream-dead source, and is
motivated to package it, they may also be willing to provide an
upstream focus.  Note that this is for new packages: once in the
archives, if upstream dies, I do not advocate removal if a new
upstream community has yet to form.

> >         (Packagers who implement get-orig-source for packages with
> > watch files get extra points)
>
> I've never found using get-orig-source to be very enlightening. For packages,
> where there is a .tar.gz, it's imho unnecessary (uscan handles this quite
> well imho), and converting e.g. .zip files to a .tar.gz looks ugly in a
> Makefile.

    I find get-orig-source preferable for three reasons: 1) It
provides a standard interface for reviewers to easily get the upstream
source, 2) for cases of content changes (non-free removals, etc.),
it's very handy to have a get-orig-source rule when preparing the next
upstream release (especially as the updater may well not be the
packager), and 3) for cases of repacking, it's nice to have it
automated rather than fussing with different methods of generating
orig.tar.gz

> >     3.  Upstream should be responsive, and maintain a bug tracker
>
> Checking for this adds an extra step to reviewing. Also I'm not entirely sure
> about the bug tracker (min12xxw, my pet package doesn't have one ;)

    Hmm..  Maybe this should be rephrased.  A bug tracker is nice, but
I agree that many upstreams don't have them.  For "responsiveness", I
generally take a glance at upstream mailing list archives, etc.  If
the entirety of activity consists of people asking questions and
reporting problems, with no visible input from the developers, I'd
class it "unresponsive", but don't tend to be more agressive about
this.  Further, most packagers are interacting with upstream, and can
readily say whether there is someone on the other end.  Of course, on
reflection, this seems related to the previous bit about upstream not
being dead: perhaps it doesn't need to be in two places.

> >     4.  Packaged version should be latest upstream
>
> [... with the exception of a good reason to not use latest upstream]

    Perhaps, although I'd argue that this requires documentation as to
the reason, to avoid many "mypony should be upgraded to verison 17.2"
bugs.  With such documentation, I would be happy to override this
guideline as a reviewer.  Without the documentation, even if I agreed,
I wouldn't think it responsible to advocate the package for inclusion.

> >     4.  Package should provide Ubuntu-specific documentation for
> > variances in behaviour from upstream
>
> Erm... usually it shouldn't vary from upstream behaviour (except of course for
> a good reason).

    Actually, we often vary from upstream in many different ways.
Upstream usually references /usr/local/* where we install to /usr/*.
We regularly create default configurations that differ from upstream,
and not infrequenty provide various launcher scripts.  (remainder of
list omitted)

    All of these tasks are part of integration into the distribution,
and should not count against the package: similarly, if any of these
changes causes a behavioral difference, it should be documented.

> >     6.  Non-native packages must have verifiable cryptographic path to
> > upstream source
>
> Does this mean that the orig.tar.gz shouldn't differ [1]?
> [1]: https://wiki.ubuntu.com/PackagingGuide/Basic#ChangingOrigTarball

    When upstream provides an orig.tar.gz, yes.  When upstream does
not, it means that there is a means by which a reviewer can 1) compare
a known upstream to a known hash value, and 2) use an automated
procedure to generate the orig.tar.gz from the verified upstream file.

> >     7.  Package must be advocated by at least two members of
> > ubuntu-dev (the packager may count as one)
>
> This doesn't adhere to the current new packages policy, where a MOTU is only
> encouraged to go through reviews, but not obliged to.

    Ah.  I misunderstood the current policy then.  I think that they
should be obligated, as I know I make mistakes, and I expect everyone
else to as well, but don't mean to be changing policy by the
presentation of the reviewing guidelines.

> > "must", as used above, indicates that a package not meeting that test
> > is not appropriate for inclusion in the archive.
> > "should", as used above, indicates that the reviewer should explicitly
> > agree to the variance from the condition prior to advocating the
> > package for inclusion in the archive.
>
> That's pretty obvious imho. Also I have a very deep aversions about any
> guidelines or especially language reference manuals (I'm looking at you, VHDL
> lrm!) which need to define what words mean. But I guess this is pretty much
> tied to my personal aversion to the aforementioned language reference manual.

    It was added after a specific query in the initial drafting
process.  I'm happy to drop it if nobody feels it's required.

-- 
Emmet HIKORY