Code review comment for lp:~npg-team/openobject-addons/assembly_bom_npg

Hello,

I would like to explain a little bit more why this merge proposal was rejected, in 2 parts: our policy for merge proposals, then some specific hints.

1. Merge Proposal Acceptance Policy
===================================
There may have been contradicting messages about how and when it is useful to make a merge proposal.
We would like to state this policy very clearly, especially now that extra-addons have been deprecated due to the introduction of OpenERP Apps. So we have now added an official Merge Proposal Acceptance Policy to our contributor documentation, please have a look: http://bit.ly/openerp-contrib-mp

2. Remarks specific to this merge proposal
==========================================
In approximate order of importance:

- based on your minimal description (it would help to have more), it looks like this module should be:
  + either a general purpose extra feature that would be a great addition for OpenERP Apps, as explained in the policy, but does not need to be included in official addons now
  + or simply a modification of the original mrp module, to make the implementation simpler and less brittle (if validated functionally)
- the size of the merge proposal diff is very large (+3000 lines) compared to the scope:
  + lot of copy/pasted code from original addons, are they all really necessary? (if yes, please make merge proposals to split the original functions into smaller, more extensible functions)
  + you included a "tiny_sxw2rml.py" script and dependent files that look totally useless and out of place (1100+ lines!)
  + some parts look quite optional or not directly related to the module's purpose, reports, etc.
- there's a cr.commit() in your code, this is unacceptable! (see coding guidelines)
- usability: no tooltips explaining what an assembly bom is and how it differs from phantom bom (on the Bom type selection)
- you should be careful when comparing computed float values, due to float rounding
- no proper description of the features in the module manifest, nor how to use them
- many useless python imports (netsvc, pooler, ...)
- multiple leftover "print" debugging statements
- some remaining commented out code lines, which should be removed

I hope this helps,

Thanks!