Control: tags -1 moreinfo

Hi Dave,

I've reviewed the package on mentors; it needs some work:

- the patch "docs and build tweaks":
  - The Bug-Debian should point to a bug _covering_ the patch, not the ITP.
    (IOW, remove that line from the dep3 patch header)
  - Maybe that patch should be sent upstream?
  - You add -Wpenandic and -g. That shouldnt be neccessary.
    (and even fragile because of the -Werror)

- Readme.Debian 
  - does not contain useful informastion, it should be probably be deleted
    - If, then this would be in Readme.source (see debian policy), but I
suggest:
    - (if you want to record the git commit id, the better place is to have it
 in the version string of the pacakge. see uscan(1) pretty-rule

- d/changelog, package version
  - The Debian revision should be "-1" for initial packages and
  - should only have this entry:
    "Initial release (Closes: #<ITP-Bug>)
  - IOW: delete the entries for -2 and -3 and make the one for -1 read that line
    above, filling in your ITP bug number
 
- d/control:
  - you only need to Build-Depend on debhelper-compat; the debhelper entry is
    redundant.
  - Standard Version can be updated. 
  - is the "!" in !Spark on purpose in the description? 

- Package sqsh has already a binary called sqsh. So you will need to rename your
  binary to something else.

- Upstream has some new commit. Maybe you could evaluate if those are nice to
have. It seems that they have added cmake support, which could be a improvement
over a simple Makefile and possibly makes (part of ) your patch obsolete and
as cmake supports install targets, it might even be able to remove the overrides
in d/rules. 

- d/snaprk-docs.docs -- I guess this file is not needed.

I'm tagging it moreinfo, that means an updated package is required to proceed
with this RFS. Once it is ready, remove the tag for a second round of review.

-- 
cheers,
tobi

Reply via email to