← Back to team overview

openerp-community team mailing list archive

Re: Community, Please pay more attention to the changes of OpenERP core

 

Hello Jeff.

We as community don't have inherence over the work done by OpenERP.

In this specific bug we are the protagonist of the commit (bad one) and the
revert (because leak of communication.

THe bug report is:

https://bugs.launchpad.net/openobject-addons/+bug/1190103

Duplicated from

https://bugs.launchpad.net/openobject-addons/+bug/1154270


The revert is due to a backguard compatibility with some belgium concepts
solved with this 0 lines.

Let share with you the answer we receive from the OPW:

***********************

Hi there,

i'd like to explain this. But first let me concede that fact that we (read
"I") have to explain this again and again is probably the normal
consequence of a poor design or something like this. If we get time to
refactore the accounting, this point will more than probably be addressed,
but anyway... it's not the subject.

These 0 lines are useful AND needed. They are really important and removing
them would lead to errors in the tax report. Indeed, in the
account.move.line object, there are 2 important concepts that are mashed up:
1) accounting stuff: debit, credit, journal and account.account..
2) tax stuff: tax amount and tax code
Both concepts are independent, and a line with debit = credit = 0 is
meaningful if the tax amount and tax code aren't empty.

Let me take a real example of a belgian tax VAT-IN-V81-21:
for a refund of 100€ using this tax, the information to consider in the tax
report are (legally, you have to provide and show these information):
*tax code 81: -100
*tax code 85: 100
*tax code 63: +21

Where the accounting stuff is:
*product account: 100
*tax account: 21

Now read me carefully: 2 lines of accounting stuff to consider, and 3 lines
of tax stuff: how can we put all this together in the same object
(account.move.line) knowing that we have on the account.move.line only a
single field for the tax code and another for the tax amount. So for a
single account.move.line, you can only record one of the three lines of my
tax stuff. Do you see my point?

That's the reason why it exists lines with debit = credit = 0, and why it
is legal in OpenERP. And if you look carefully on the video given in the
bug report, it appears that the "0-line" pointed is clearly having tax
information on it (tax code and tax amount columns are filled). In our
example, OpenERP will more likely create something looking like this:
*product account: 0, tax code 81: -100
*product account: 100, tax code 85: 100
*tax account: 21, tax code 63: +21
But as i said, the 2 concepts are independent and you'd have the same
results in your tax report with
*product account: 0, tax code 85: 100
*product account: 100, tax code 63: 21
*tax account: 21, tax code 81: -100

So i hope, i made it clear. I'm gonna ask to update the status of the bug
reports into the proper state (invalid) with a proper explanation, + the
revert of patch 9265 that was by mistake accepted on stable version
(although it was only hiding 0-lines, not preventing their appearance i do
not see any good reason to accept this hack anyway).

Thanks for your comprehension
Quentin

*************************

We decide internally take the Quentin's answer as valid and we will improve
our localizations until this behavior can be refactored.

FYI:

We report the bug in June, and we exchange a lot of discusions with OpenERP
support team, sometimes is not humanly possible mantain everything updated.

II ask apologize because i think you are right, but it is better manage
with bzr pull instead update qith nightly it give you more control.

Just as experience, You are right, but this kind of bugs will continue
happening I think the request should be re-directed to OpenERP SA if they
share the "conclusion" of the OPW report it can help a lot.

BUT BTW it is a known issue and we need to be prepared, as you saw it was
not a light decision it was so discussed __before__ and after the merge, I
think this things happend even in the best families :-)

IMHO it brings "more" problems NOT MERGE As soon as the problem is solved,
because everyone of us can not see all the solutions, but the improvement
should be just copy and pasting the "conclusion" of the solution in the
commit message for now.

We have several examples where we don't ask for the merge until we test
internally very well, but it is far from 15 minutes my friend.

Regards, I hope it helps you understand this specific case.


2013/10/9 jeff.wang <jeff@xxxxxxxxx>

> Hi All,
>
>    I am very glad to see that may community projects cover great modules
> become more active, but I really worry about the core package now.
>
>     We know, the package openerp.com build every night is the first sight
> our potential customers know what OpenERP looks like. the quanlity of this
> part shouldn't be technically "not dump in runbot", it should be
> functionally "every changes are reasonable". So every community user should
> pay more attentions to the changes of openerp addons/server/web  everyday,
> "more eyeballs, less bugs", correct me if I am wrong. it costs about 10-15
> minutes for a quick code review, but openerp community's future need this
> time funding from each member.
>
>     I write this mail because I begin to review the changes everyday from
> 2 months ago, I do found a problem:
>
>      " Not all merged commit bind to a specific bug report, especial the
> opw ones."
>
>     When I see that change, I have to guess what happened without this
> change. 50% times I can know that by some 10 minutes research, 50% times I
> don't know why this change is introduced. by the way, I have to say Oliver
> Dony always show best practice in the commit comments: 5W1H, and also the
> commits bind to specific bug, I can see who report it, what is the result
> he want, what is the result he got, how to reproduce at other's side...
> after all, I have to say, review the code changes in this 2 months cost me
> more time than I think it should.
>
>      give you a sample, yesterday we have this change
>
>
> http://bazaar.launchpad.net/~openerp/openobject-addons/7.0/revision/9510
>
>      [FIX] account: revert commit 9265, show every invoice line in the
> list view and do not force domain on it to remove 0 debit/credit lines
>
>      so I go to
>
> http://bazaar.launchpad.net/~openerp/openobject-addons/7.0/revision/9265
>     [MERGE] [FIX] OPW 590066 account: remove accounting information lines
> from Journal items
>
>     You know my English is very poor, I really cannot understand if
> "remove accounting information lines" equal to "remove 0 debit/credit lines"
>
>     I just want to know the reason why this domain need this
> ['|',['debit', '!=', 0], ['credit', '!=', 0]] value from 2013-06-24 and
> never need it from 2013-10-08.  actually I want to know what opw 590066
> says 3 months ago and why it's wrong yesterday.
>
>     Furthermore, we ALL are building solution to end users base on openerp
> core function, keep every merge to this repo touch everybody's eyeball,
> IMPO this is the open soure way.  for developers, I suggest every merge
> request should link a bug,  if there is none, create one. so that at least
> somebody can discuss this change later, or ask for an explain about why
> this change is needed.
>
> ------------------
> Jeff Wang |  jeff@xxxxxxxxx | 18016291663 | 02158980787
> @OpenERP_Jeff "As simple as possible, As complex as needed"
>   <http://www.osbzr.com>
>  Maintainer of Open ERP china community
>  http://www.openerp-china.org
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>
>


-- 
--------------------
Saludos Cordiales

Nhomar G. Hernandez M.
+58-414-4110269
Skype: nhomar00
Web-Blog: http://geronimo.com.ve
Servicios IT: http://vauxoo.com
Linux-Counter: 467724
Correos:
nhomar@xxxxxxxxxxxxxx
nhomar@xxxxxxxxxx
twitter @nhomar

Follow ups

References