Hello Lucio Correia,

On Thu, Dec 15, 2016 at 02:58:46PM -0200, Lucio Correia wrote:
> Breno,
> 
> I've upload a new package with requested fixes.
> 
> dget -x https://mentors.debian.net/debian/pool/main/w/wok/wok_2.3.0-1.dsc
> 
> Let me know if anything else is necessary for wok.

I took a quick look at this package and here are a few notes I made.
I don't personally intend to sponsor you as this seems to be outside
my expertice.

- Seems like you've made an effort both to remove and use/depend
  on packaged javascript components like jquery etc, as well as 
  adding missing sources for the minified bundled things.
  This should avoid the need for repackaging the orig tarball
  to remove source-less files, but at the same time it's a bit hard
  to check if you catched everything. Also, while I guess it's not
  strictly forbidden to bundle javascripts that aren't already packaged
  separately I think it's atleast frowned upon. You might want to
  consider joining in on the javascript packaging efforts and get
  (all of) your javascript dependencies packaged separately.

- postinst pokes around in /etc and that is always a source for finding
  policy violating / release critical bugs. For example you check
  only for configure stage and enables the nginx site. This means
  you'll likely trigger on package upgrades and reenable a site that
  the local administrator might have disabled. Fix would likely
  be to check $2 for version to detect if configure is running under
  upgrade or newly-installed-package.

- postrm removes files without checking ownership.
  I think it's ok for the purge action to be brutal, but the remove
  action should under no conditions cause harm to potentially locally
  configured things. For example you better check if sites-enable/wok
  is a symlink before removing it. The local admin might have
  replaced any of the files you installed under /etc with his own
  so removing them without very careful checking is likely to not
  end well.

- consider dropping the conditional clauses for invoke-rc.d in both
  your postinst and postrm. The invoke-rc.d has been around for many
  releases and should always be available (or it would be a bug in
  the base system). I'm aware policy contains similar to what you're
  doing but policy is simply severely outdated. Directly invoking
  an init.d script can be potentially harmful and should never
  be done in todays system. (Policy even explicitly forbids it.)

- 0001-Do-not-install-firewalld-conf-in-Debian.patch
  why? Please always document why.
  (fwiw, firewalld is available in debian also.)

- 0001-Rename-wokd-to-wok.patch
  why?

- 0001-Update-Datatables-Switch-and-Editable-libraries.patch 
    patch is full of trailing whitespace addition noise.

- 0005-Add-nginx.service-as-wokd.service-dependency.patch
  Executing kill in ExecStop is bad as it's async. This is bad
  and potentially causes 'restart' to fail as 'stop' action
  will not wait for stop to actually finish.


HTH.

Regards,
Andreas Henriksson

PS. you might want to block ginger RFS on ginger-base RFS and
    ginger-base RFS on wok RFS.

Reply via email to