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

Re: [RFS] mseed2sac and sac2mseed: seismic data conversion tools



Dear Paride,

On Wed, Jan 17, 2018 at 11:57:05AM +0100, Paride Legovini wrote:
> On 2018-01-12 14:13, Paride Legovini wrote:
> > Dear debian-scientists,
> > 
> > I packaged two small utilities to convert seismic data between the two
> > most commonly used data formats. Both the tools use the recently
> > packaged libmseed2 library (thanks Pierre).
> > 
> > Here are the ITPs:
> > 
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=886993
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=886995
> > 
> > and the uploads to mentors:
> > 
> > https://mentors.debian.net/package/mseed2sac
> > https://mentors.debian.net/package/sac2mseed
> 
> I updated the packages. The repositories are linked in the ITPs, I'll
> copy the links here for convenience:
> 
> https://salsa.debian.org/paride-guest/mseed2sac
> https://salsa.debian.org/paride-guest/sac2mseed
> 
> Checkout the "debian/sid" branch.
> 
> Still looking for reviews and sponsorship :)

I just looked at mseed2sac, but maybe the remarks below also apply to
sac2mseed:

- the Vcs-Git and Vcs-Browser fields in debian/control should point to the
  Debian packaging (on salsa.debian.org), not to the upstream sources

- it looks like you repackaged the original tarball by removing the libmseed/
  directory. This should be made clear in the Debian version by adding a suffix
  (typically "+ds", for "Debian source", so that the Debian version will be
  "2.2+ds-1").
  Then you also want to update debian/watch to take into account the +ds
  suffix (with the "dversionmangle" option), and also debian/copyright (by
  using the Files-Excluded field) to make repacking easy (see the manpage for
  "uscan" for both points)

- please add a "pristine-tar" branch, we rely on it in the Debian Science Team
  (see the Debian Science Policy,
  https://debian-science.alioth.debian.org/debian-science-policy.html)

- please also use standard git-buildpackage branch names, i.e. "upstream" for
  upstream sources and "master" for Debian packaging (this naming scheme is
  expected in the Debian Science Team). Actually, we usually do not track
  upstream's git, but rather rely on upstream tarballs.

- why do you add -O3 in debian/rules? Unless you have a good reason (like it
  avoids a build failure), you are expected to use standard Debian build flags
  (currently the default optimization is -O2). Incidentally, your modification
  probably breaks non-optimized build (triggered with DEB_BUILD_OPTIONS=noopt),
  since -O3 will be enforced even in that case.

Otherwise your packaging looks good (though I can't promise that there are no
other issues that may be discovered in a second round of review).

Best,

-- 
⢀⣴⠾⠻⢶⣦⠀  Sébastien Villemot
⣾⠁⢠⠒⠀⣿⡁  Debian Developer
⢿⡄⠘⠷⠚⠋⠀  http://sebastien.villemot.name
⠈⠳⣄⠀⠀⠀⠀  http://www.debian.org

Attachment: signature.asc
Description: PGP signature


Reply to: