Code review comment for lp://staging/~inddiana/sisb/jose_l10n_ve_withholding_islr_final

El 31 de marzo de 2012 10:57, [SISB] Aristóbulo Meneses <
<email address hidden>> escribió:

> > Por otra parte:
> >
> > A nivel técnico:
> >
> > 51 + def create(self,cr,uid,values,context=None):
> > 52 + if values.get('type',False) == 'consu':
> > 53 + raise osv.except_osv(('Error !'), ('No puede crear un producto
> como
> > consumible !'))
> > 54 + if values['purchase_requisition'] == True:
> > 55 + raise osv.except_osv(('Error !'), ('Debe marcar la opcion
> solicitud
> > de compra !'))
> > 56 + return super(product_product,
> > self).create(cr,uid,values,context=None)
> >
> > Esto es una mala práctica, los raise se deben _evitar_ en los creates al
> menos
> > que estés validando errores de consistencia, es SIEMPRE preferible un
> > contraint para lo que estás evaluando acá.
> >
>
> En este punto tienes toda la razón.
>
> > Otro punto:
> >
> > Por que copias tooooodo el módulo y simplemente no heredan del original
> > nuestro..... recuerden que nosotros lo tenemos corriendo en varios
> sitios que
> > le vamos encontrando bugs día a día, creo que es mejor de esa manera o
> que
> > bombardeen la localizacion con bugs + patchs, así creamos uno más fuerte
> y
> > documentado, el copy tu pasteo es pan para hoy hambre para mañana (ya
> nos ha
> > pasado.)
> >
> >
>
> Simple, nuestro módulo de retención va a tomar otro enfoque muy distinto
> al que actualmente tiene el módulo de ustedes. En estos tres meses de
> experiencias con el módulo hemos conseguido muchas diferencias que tienen
> mas que ver a que sin duda ustedes partieron de las necesidades y
> procedimientos muy particulares de una empresa, asumiendo siempre los
> mejores escenario posibles. Al final la ley es una sola y la matemática es
> la misma.
>

Pero si no explicas TU enfoque no sabremos que tenemos mal nosotros, y no
sumas para acá y restas para allá, por que forkear algo que tiene ya
aplicado VARIOS años de experiencia, y reescrito 3 veces dicho sea de paso,
con 3 meses de revisiones no continuas, me parece que estarás cometiendo un
error.

Te doy un ejemplo de nuestra experiencia:

Nosotros forkeamos VOUCHER en nuestra primera implementación, por
EXACTAMENTE la misma razon que me estás dando _supuestamente_ nosotros
teníamos razones para hacerlo, pero jamás las publicamos.

Cuando salió la versión 6 tuvimos que reescribir TOOOOOODO nuestro módulo y
unirlo al trunk por que TODAS las decisiones que tomamos en arquitectura,
estabas REPETIDAS en el nuevo módulo de OpenERP, entonces, conclusión,
Nosotros perdimos el trabajo, ganamos experiencia, y OpenERP siguió su
camino por separado avanzando 50x más rápido que nosotros.

Hoy: Cuando NO entendemos o NO estamos de acuerdo con OpenERP, invertimos
en propuestas de merge al core y discutir públicamente MIS por ques, en el
90% de los casos hemos ganado, o el resultado ha sido un módulo 100%
compatible con futuras versiones.

Ésto es lo que intenta decir Humberto, creo que la decisión es demasiado
viceral solo forkear por que le darás OTRO enfoque -sin explicar el
enfoque- escribiendolo, en unos meses, -decidido o no- tendremos sustentos
más sólidos-

No cometas errores del pasado , los forks son siempre malos, y más aún
cuando el Papá del desarrollo está trabajando contigo, en tu idioma, y a tu
ritmo....

Sigo insistiendo, es un error.

>
> > Propongo_ crear en SISB un módulo que se llame igual con sufijo: EXT (de
>
>

> NO
>

PS: Se ve feo un NO por que me da la gana, recuerda que lo mejor es
explicarse, así en el camino de la escritura sustentarás tus porques, con
documentación sólida.

Estás cometiendo EL MISMO error que han cometido muchos en la Comunidad SL,
pero ésta vez en menor escala.... aprender de los errores de otros no está
mal.

Saludos mi pana.

>
> > extendido) para poder estabilizarlo y portar lo s cambios poco a poco si
> > quieren mantener uno propio o:
> >
> > HAcer tantas propuestas de merge como cambios requieran, si se fijan:
> >
> > Toda nuestra localización pasa cliente a cliente por una lista como ésta
> de
> > validaciones:
> >
> > http://yfrog.com/h4au2ebj
> >
> > Si ustedes comparten una extensión de ñesta antes o despues de probar,
> > avanzaríamos más rápido.
> >
> > Saludos y disculpen lo largo.....
> --
>
> https://code.launchpad.net/~inddiana/sisb/jose_l10n_ve_withholding_islr_final/+merge/100228<https://code.launchpad.net/%7Einddiana/sisb/jose_l10n_ve_withholding_islr_final/+merge/100228>
> You are reviewing the proposed merge of
> lp:~inddiana/sisb/jose_l10n_ve_withholding_islr_final into lp:sisb.
>