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

Bug#1055041: RFS: a2d/2.0.3-1 -- APRS to DAPNET portal



Control: tags -1 moreinfo

Hi Yogeswaran,



>   dget -x https://mentors.debian.net/debian/pool/main/a/a2d/a2d_2.0.3-1.dsc
> 
> Changes since the last upload:
> 
>  a2d (2.0.3-1) unstable; urgency=medium
>  .
>    * New upstream version.
>    * Fixed failed VCS repository. (Closes: #1055040)

I've took a look at your package:


- You do not replace the debian changelog, you append the new
  information. (hint: use "debchange" or "dch" for changelog manipulation)

- there are undocumented changes, for example it seems that dependencies
  has been changeed. that (and why it is) should be noted in
  d/changelog.


(You are also upstream)
- It seems you have renamed a2dapp to a2d. This change is not good,
  as it is too short / generic, and renaming is a breaking change
  for your users. I'd appreaciate if you could stay with the old name.

- Do you really need an hard dependency on nginx?
  (I've checked myself: I think you don't. you'll use nginx as a proxy,
  but people might want to setup e.g another http server for that or
  even directly connect to the 9333 port (can they?); I think nginx
  should be Recommends: not Depends: -- see policy about the definition
  of Depends.)

- You README.md suggests to do rm -rf <various directories>
  That is bad advice! Do not blindly say that they should nuke the nginx
  configuration; (If, then the user should run apt purge nginx, but this
  is also still dangerous a people might loose data); deluser is also
  a bad advice, (system) users should not be deleted after they have
  been created.

- postinst is wrong. Especially you do not remove those files on
  "remove" -- only purge should delete user configuration, for example.

Thinking about it I am not sure if you are handling conffiles correctly,
as your postrm will nuke also files not handled by your packge, by
blindly removing them. This is an RC bug. Probably your application
will create files there? Maybe than /etc/ is the wrong location, maybe
app-created files should be reside in /var/lib/a2d? I'm not sure here
if/how this is handled correctly, maybe someone else will chime in.
I'm just sure that "rm -r /etc/a2d" could make some admins unhappy.

(I'm running out of time here, the review might not be complete)

--
tobi


> Regards,
> -- 
>   Yogeswaran Umasankar
> 


Reply to: