Review of Accounting/Budget functionality via r1573884 & r1574400

[Pierre Smits <pi...@gmail.com>]

Hi All,

I finally had some time to review the functionality and code submitted
under:

   - http://svn.apache.org/r1573884
   - http://svn.apache.org/r1574400

The functionality is available in the demo environment on
https://ofbiz-vm.apache.org:8443/accounting

The code is related to OFBIZ-3169
<https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the
committer (Hans Bakker) didn't feel the need to follow due process by
adding the improvement as a patch for review by other community members
before dumping it into trunk.
Though it may seem that missing budget functionality can be regarded as a
bug, it should be considered an improvement and therefore it should follow
the Review-then-Commit principle in stead of Commit-then-Review.

Functionality overview:
Basically the functionality allows users of the the accounting component to:

   - search and find existing budget request
   - create a new budget request
   - enhance the budget request with
      - budget items
      - budget request roles
      - budget request reviews
   - change the status of the budget request

Findings:
First of all, documentation describing the functionality and how to use it
is not available. This is a mayor omission. Assessment of usability by end
users is hindered by this lacking.

Secondly, the functionality as presented works. No errors occurred when
clicking on the various buttons and executing basic process steps, like:

   - adding removing budget items
   - adding and removing a review
   - adding and removing a role
   - Changing the status of the budget request

But, can we say that this functionality is complete and lifts OFBiz in
general and the accounting module in particular a step higher on the ladder
towards best of breed in the category 'Open Source ERP Projects c.q.
Solutions'?

It definitely does not. The functionality appears, though included in the
accounting module, to be an island. Budgets in general (and at least), are
related to organisations (in OFBiz the internal organisations), P&L
centers, gl accounts, and persons. This is lacking in the functionality. No
internal organisation can be assigned, nor a P&L center or at the lowest a
gl account. Budgets without such associations are meaningless.

No permissions where incorporated in menu-items to ensure that
functionalities can only be executed by appropriate users. No roles were
predefined, which is customarily applicable regarding this kind of
functionality. Not every users having access should be able to create
budget request, or perform subsequent actions.

Lacking this, the user can (must?) select any partyId and roleId to
associate with the budget request. Even parties not working with (or
allowed to work with) OFBiz in general and the accounting module in
particular can be selected.
But also due to the lacking possibility to set the internal organisation,
no default currency is set. Thus, making the amounts ambiguous to interpret.

Furthermore it provides only a basic workflow process consisting of
(approve, reject and review). Associating a party to a budget request
doesn't warrant that others can not manipulate the content/data associated
with the request. When I approved a budget request, I had the button to
reject available. This shouldn't be possible. And at one occasion I could
click the 'reject' button multipe time to get to the final state.
While status changes are registered, it should also show who invoked the
status change.

Due to the fact that arbitrarily any party can be associated to a budget
request without any restrictions, the workflow is rendered meaningless.

Conclusion(s):
This solutions hasn't been thought through properly and does no good for
the quality of OFBiz (both the project and the product) in general and for
the accounting module in particular. Prior to dumping the code into trunk
it should have been posted for review by the community, so this could have
been avoided.

If any other contributor to this project made such a set of functionalities
available as a patch to an OFBiz issue in JIRA, it would sit there for a
great length of time before it would have been committed.

I advice to remove it from trunk.

Regards,

Pierre Smits

*ORRTIZ.COM <http://www.orrtiz.com>*
Services & Solutions for Cloud-
Based Manufacturing, Professional
Services and Retail & Trade
http://www.orrtiz.com

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Scott Gray <sc...@hotwaxmedia.com>]

Typo: "design designs" -> "design decisions"

Regards
Scott

On 10/08/2014, at 11:25 am, Scott Gray <sc...@hotwaxmedia.com> wrote:

> The Budget data model has been in place for over 8 years, someone finally makes an attempt to build it out a little with some basic screens, services and data and you want that reverted?  
> 
> This is a step in the right direction, nothing more.  If you expect every improvement to be some sort of a step across the feature finish line then you're going to be disappointed over and over again because the community has never worked that way.
> 
> The only reason for a revert is when the step is in the wrong direction and will make working towards the finish line more difficult for future contributions.  I cannot fathom how that would be the case here given that the implementation are basic and virtually no design designs have been made (aside from a few StatusItem records that can easily be changed/added to).
> 
> Regards
> Scott
> 
> On 20/06/2014, at 9:54 am, Pierre Smits <pi...@gmail.com> wrote:
> 
>> Adrian,
>> 
>> Thank you for your opinion. And for lowering the bar.
>> 
>> This code dump isn't a workable solution for - as a caption on the OFBiz
>> website states - 'the best e-commerce and Enterprise Resource Planning
>> (ERP) software available.
>> 
>> Instead it is a waste of time and an insult to everybody who designs and
>> develops good, thought-through business solutions for OFBiz, and you should
>> be offended to.
>> 
>> Did Hans hand over his committers userId and password to one of his junior
>> developers or the intern of the week? Because this is definitely not even
>> close to the same level of quality and coherence as the project mgt or
>> scrum solution. And if he didn't, it can surely not be regarded as 'going
>> the extra mile' visavis completeness or working with the community.
>> 
>> 
>> 
>> Pierre Smits
>> 
>> *ORRTIZ.COM <http://www.orrtiz.com>*
>> Services & Solutions for Cloud-
>> Based Manufacturing, Professional
>> Services and Retail & Trade
>> http://www.orrtiz.com
>> 
>> 
>> On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum <
>> adrian.crum@sandglass-software.com> wrote:
>> 
>>> I don't see why it needs to be reverted. If it needs to be built out more,
>>> then devs can supply patches for that.
>>> 
>>> There have been many times in the past where some basic functionality is
>>> introduced to the project, and then it is up to the rest of the community
>>> to finish building it out.
>>> 
>>> Adrian Crum
>>> Sandglass Software
>>> www.sandglass-software.com
>>> 
>>> 
>>> On 6/19/2014 5:26 AM, Pierre Smits wrote:
>>> 
>>>> Hi All,
>>>> 
>>>> I finally had some time to review the functionality and code submitted
>>>> under:
>>>> 
>>>>   - http://svn.apache.org/r1573884
>>>>   - http://svn.apache.org/r1574400
>>>> 
>>>> 
>>>> The functionality is available in the demo environment on
>>>> https://ofbiz-vm.apache.org:8443/accounting
>>>> 
>>>> The code is related to OFBIZ-3169
>>>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the
>>>> 
>>>> committer (Hans Bakker) didn't feel the need to follow due process by
>>>> adding the improvement as a patch for review by other community members
>>>> before dumping it into trunk.
>>>> Though it may seem that missing budget functionality can be regarded as a
>>>> bug, it should be considered an improvement and therefore it should follow
>>>> the Review-then-Commit principle in stead of Commit-then-Review.
>>>> 
>>>> Functionality overview:
>>>> Basically the functionality allows users of the the accounting component
>>>> to:
>>>> 
>>>>   - search and find existing budget request
>>>>   - create a new budget request
>>>>   - enhance the budget request with
>>>>      - budget items
>>>>      - budget request roles
>>>>      - budget request reviews
>>>>   - change the status of the budget request
>>>> 
>>>> 
>>>> Findings:
>>>> First of all, documentation describing the functionality and how to use it
>>>> is not available. This is a mayor omission. Assessment of usability by end
>>>> users is hindered by this lacking.
>>>> 
>>>> Secondly, the functionality as presented works. No errors occurred when
>>>> clicking on the various buttons and executing basic process steps, like:
>>>> 
>>>>   - adding removing budget items
>>>>   - adding and removing a review
>>>>   - adding and removing a role
>>>>   - Changing the status of the budget request
>>>> 
>>>> 
>>>> But, can we say that this functionality is complete and lifts OFBiz in
>>>> general and the accounting module in particular a step higher on the
>>>> ladder
>>>> towards best of breed in the category 'Open Source ERP Projects c.q.
>>>> Solutions'?
>>>> 
>>>> It definitely does not. The functionality appears, though included in the
>>>> accounting module, to be an island. Budgets in general (and at least), are
>>>> related to organisations (in OFBiz the internal organisations), P&L
>>>> centers, gl accounts, and persons. This is lacking in the functionality.
>>>> No
>>>> internal organisation can be assigned, nor a P&L center or at the lowest a
>>>> gl account. Budgets without such associations are meaningless.
>>>> 
>>>> No permissions where incorporated in menu-items to ensure that
>>>> functionalities can only be executed by appropriate users. No roles were
>>>> predefined, which is customarily applicable regarding this kind of
>>>> functionality. Not every users having access should be able to create
>>>> budget request, or perform subsequent actions.
>>>> 
>>>> Lacking this, the user can (must?) select any partyId and roleId to
>>>> associate with the budget request. Even parties not working with (or
>>>> allowed to work with) OFBiz in general and the accounting module in
>>>> particular can be selected.
>>>> But also due to the lacking possibility to set the internal organisation,
>>>> no default currency is set. Thus, making the amounts ambiguous to
>>>> interpret.
>>>> 
>>>> Furthermore it provides only a basic workflow process consisting of
>>>> (approve, reject and review). Associating a party to a budget request
>>>> doesn't warrant that others can not manipulate the content/data associated
>>>> with the request. When I approved a budget request, I had the button to
>>>> reject available. This shouldn't be possible. And at one occasion I could
>>>> click the 'reject' button multipe time to get to the final state.
>>>> While status changes are registered, it should also show who invoked the
>>>> status change.
>>>> 
>>>> Due to the fact that arbitrarily any party can be associated to a budget
>>>> request without any restrictions, the workflow is rendered meaningless.
>>>> 
>>>> Conclusion(s):
>>>> This solutions hasn't been thought through properly and does no good for
>>>> the quality of OFBiz (both the project and the product) in general and for
>>>> the accounting module in particular. Prior to dumping the code into trunk
>>>> it should have been posted for review by the community, so this could have
>>>> been avoided.
>>>> 
>>>> If any other contributor to this project made such a set of
>>>> functionalities
>>>> available as a patch to an OFBiz issue in JIRA, it would sit there for a
>>>> great length of time before it would have been committed.
>>>> 
>>>> I advice to remove it from trunk.
>>>> 
>>>> Regards,
>>>> 
>>>> Pierre Smits
>>>> 
>>>> *ORRTIZ.COM <http://www.orrtiz.com>*
>>>> 
>>>> Services & Solutions for Cloud-
>>>> Based Manufacturing, Professional
>>>> Services and Retail & Trade
>>>> http://www.orrtiz.com
>>>> 
>>>> 
> 

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Scott Gray <sc...@hotwaxmedia.com>]

The Budget data model has been in place for over 8 years, someone finally makes an attempt to build it out a little with some basic screens, services and data and you want that reverted?  

This is a step in the right direction, nothing more.  If you expect every improvement to be some sort of a step across the feature finish line then you're going to be disappointed over and over again because the community has never worked that way.

The only reason for a revert is when the step is in the wrong direction and will make working towards the finish line more difficult for future contributions.  I cannot fathom how that would be the case here given that the implementation are basic and virtually no design designs have been made (aside from a few StatusItem records that can easily be changed/added to).

Regards
Scott

On 20/06/2014, at 9:54 am, Pierre Smits <pi...@gmail.com> wrote:

> Adrian,
> 
> Thank you for your opinion. And for lowering the bar.
> 
> This code dump isn't a workable solution for - as a caption on the OFBiz
> website states - 'the best e-commerce and Enterprise Resource Planning
> (ERP) software available.
> 
> Instead it is a waste of time and an insult to everybody who designs and
> develops good, thought-through business solutions for OFBiz, and you should
> be offended to.
> 
> Did Hans hand over his committers userId and password to one of his junior
> developers or the intern of the week? Because this is definitely not even
> close to the same level of quality and coherence as the project mgt or
> scrum solution. And if he didn't, it can surely not be regarded as 'going
> the extra mile' visavis completeness or working with the community.
> 
> 
> 
> Pierre Smits
> 
> *ORRTIZ.COM <http://www.orrtiz.com>*
> Services & Solutions for Cloud-
> Based Manufacturing, Professional
> Services and Retail & Trade
> http://www.orrtiz.com
> 
> 
> On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum <
> adrian.crum@sandglass-software.com> wrote:
> 
>> I don't see why it needs to be reverted. If it needs to be built out more,
>> then devs can supply patches for that.
>> 
>> There have been many times in the past where some basic functionality is
>> introduced to the project, and then it is up to the rest of the community
>> to finish building it out.
>> 
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>> 
>> 
>> On 6/19/2014 5:26 AM, Pierre Smits wrote:
>> 
>>> Hi All,
>>> 
>>> I finally had some time to review the functionality and code submitted
>>> under:
>>> 
>>>    - http://svn.apache.org/r1573884
>>>    - http://svn.apache.org/r1574400
>>> 
>>> 
>>> The functionality is available in the demo environment on
>>> https://ofbiz-vm.apache.org:8443/accounting
>>> 
>>> The code is related to OFBIZ-3169
>>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the
>>> 
>>> committer (Hans Bakker) didn't feel the need to follow due process by
>>> adding the improvement as a patch for review by other community members
>>> before dumping it into trunk.
>>> Though it may seem that missing budget functionality can be regarded as a
>>> bug, it should be considered an improvement and therefore it should follow
>>> the Review-then-Commit principle in stead of Commit-then-Review.
>>> 
>>> Functionality overview:
>>> Basically the functionality allows users of the the accounting component
>>> to:
>>> 
>>>    - search and find existing budget request
>>>    - create a new budget request
>>>    - enhance the budget request with
>>>       - budget items
>>>       - budget request roles
>>>       - budget request reviews
>>>    - change the status of the budget request
>>> 
>>> 
>>> Findings:
>>> First of all, documentation describing the functionality and how to use it
>>> is not available. This is a mayor omission. Assessment of usability by end
>>> users is hindered by this lacking.
>>> 
>>> Secondly, the functionality as presented works. No errors occurred when
>>> clicking on the various buttons and executing basic process steps, like:
>>> 
>>>    - adding removing budget items
>>>    - adding and removing a review
>>>    - adding and removing a role
>>>    - Changing the status of the budget request
>>> 
>>> 
>>> But, can we say that this functionality is complete and lifts OFBiz in
>>> general and the accounting module in particular a step higher on the
>>> ladder
>>> towards best of breed in the category 'Open Source ERP Projects c.q.
>>> Solutions'?
>>> 
>>> It definitely does not. The functionality appears, though included in the
>>> accounting module, to be an island. Budgets in general (and at least), are
>>> related to organisations (in OFBiz the internal organisations), P&L
>>> centers, gl accounts, and persons. This is lacking in the functionality.
>>> No
>>> internal organisation can be assigned, nor a P&L center or at the lowest a
>>> gl account. Budgets without such associations are meaningless.
>>> 
>>> No permissions where incorporated in menu-items to ensure that
>>> functionalities can only be executed by appropriate users. No roles were
>>> predefined, which is customarily applicable regarding this kind of
>>> functionality. Not every users having access should be able to create
>>> budget request, or perform subsequent actions.
>>> 
>>> Lacking this, the user can (must?) select any partyId and roleId to
>>> associate with the budget request. Even parties not working with (or
>>> allowed to work with) OFBiz in general and the accounting module in
>>> particular can be selected.
>>> But also due to the lacking possibility to set the internal organisation,
>>> no default currency is set. Thus, making the amounts ambiguous to
>>> interpret.
>>> 
>>> Furthermore it provides only a basic workflow process consisting of
>>> (approve, reject and review). Associating a party to a budget request
>>> doesn't warrant that others can not manipulate the content/data associated
>>> with the request. When I approved a budget request, I had the button to
>>> reject available. This shouldn't be possible. And at one occasion I could
>>> click the 'reject' button multipe time to get to the final state.
>>> While status changes are registered, it should also show who invoked the
>>> status change.
>>> 
>>> Due to the fact that arbitrarily any party can be associated to a budget
>>> request without any restrictions, the workflow is rendered meaningless.
>>> 
>>> Conclusion(s):
>>> This solutions hasn't been thought through properly and does no good for
>>> the quality of OFBiz (both the project and the product) in general and for
>>> the accounting module in particular. Prior to dumping the code into trunk
>>> it should have been posted for review by the community, so this could have
>>> been avoided.
>>> 
>>> If any other contributor to this project made such a set of
>>> functionalities
>>> available as a patch to an OFBiz issue in JIRA, it would sit there for a
>>> great length of time before it would have been committed.
>>> 
>>> I advice to remove it from trunk.
>>> 
>>> Regards,
>>> 
>>> Pierre Smits
>>> 
>>> *ORRTIZ.COM <http://www.orrtiz.com>*
>>> 
>>> Services & Solutions for Cloud-
>>> Based Manufacturing, Professional
>>> Services and Retail & Trade
>>> http://www.orrtiz.com
>>> 
>>> 

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

On the other hand degrading the software quality of OFBiz is not good as well.
For instance, did someone ever get the "new" Product "Images Management" to work? how to upload, etc. ?

When someone new to OFBiz, like Ron recently, look at this feature, try to use it, see it does work not as expected and can't find any information 
about it, s/he can reasonably doubt about the whole quality of OFBiz.
Of course you, I and others could fill the missing pieces. But we are not even sure this has even worked one day (It has surely but obviously it's no 
longer OOTB).

Recently we decided about slimming down OFBiz. I see no reasons to allow committers to dump botched work in OFBiz if this means another slimming down 
session in few years :(
Even more it these committers don't care at all after having dumped their crap!

Thank you :/

Jacques

Le 20/06/2014 18:32, Adrian Crum a écrit :
> To clarify: I wasn't trying to push for a particular methodology. I was trying to address Pierre's approach to this community, where he demands that 
> any code he doesn't like be reverted (he has done this before). Traditionally, that is not how we do things in this community.
>
> I agree that a commit that breaks the build process or severely impacts the operation of the application should be fixed as quickly as possible or 
> it should be reverted. That is not the case in this thread. The new functionality might not meet Pierre's standards, but it isn't breaking the 
> build. Therefore, I don't agree that it should be reverted.
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
> On 6/20/2014 9:01 AM, Adam Heath wrote:
>> On 06/20/2014 10:42 AM, Adrian Crum wrote:
>>> No one is lowering the bar. The problem is, you still don't understand
>>> how an open source community works.
>>
>> This is wrong.  There is no hard-set one-workflow-to-bind-them
>> methodology here.  Some people seem to think there is.
>>
>> The Review-Commit workflow people sometimes want all stuff reviewed.
>> While that can help find many issues ahead of time, it will never be
>> able to find and implement all corner cases.  That can generally only
>> happen when the isolated code is pushed into the wider community.  At
>> some point, you just need to push your reviewed code to someone else,
>> and continue to fix/improve afterwards.
>>
>> The Commit-Review workflow can sometimes cause severe breakage for other
>> users.  If not-ready code is pushed into trunk, and it causes feature
>> breakage, or compilation problems, or corruption, the community at large
>> might not be able to help, as the newly pushed code is unknown, and
>> it'll take a while to come up to speed.
>>
>> Speaking about one earlier point: Committing code *early* does not lower
>> the bar for the committed code.  Code is code.  It is, or it isn't. When
>> something is committed has no bearing on the quality of the commit.
>> That quality is a separate item from the time of the commit.
>>
>>> Let me give you an example:
>>>
>>> I helped design the custom XML file format OFBiz uses for UI labels
>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people
>>> in the community who disagreed with the design, but it was committed
>>> anyway.
>>
>> Even here, you didn't do this initial work *in trunk*.  You thought
>> about the idea for a while, tried some things, got an initial set, then
>> eventually committed to trunk.
>>
>>> For the next few months, there were a lot of commits to fix bugs that
>>> cropped up after the initial commit. Scott and David helped with the
>>> bug fixes and improvements.
>>>
>>> Eventually, the new feature was working well - but there were still
>>> hundreds of properties files that needed to be converted to the new
>>> format. That was done over a period of several years by many different
>>> people.
>>
>> Another concrete example.
>>
>> Currently, I'm working on fixes to the entityengine crypt system.
>>
>> 2 years ago, I implemented key-encrypting-key support(read up on the
>> credit card number security docs).  I worked on the code in isolation,
>> on behalf of a customer.  Once it worked there, I then directly pushed
>> to ofbiz trunk.  This is the Commit-Review workflow.
>>
>> No review happened.  None.  If such review had happened, it might have
>> discovered that direct lookup of encrypted fields would have been broken
>> by my new code(a random salt is prepended to each encrypted value, which
>> prevents lookups).
>>
>> Even if that review *had* happened, after the commit, or even *before*
>> the commit, and I didn't add that random salt, it *still* wouldn't have
>> fixed the direct lookup problem.  This was due to direct-lookup being
>> broken as far back as release4.0, and probably even all the way back to
>> 2005(!).  This points to a general lack of review, at *all* levels.
>>
>> It's been my experience, that the Review-Commit workflow will get you
>> some small about of early reviews; those people who are keenly
>> interested in your new idea will possibly wake up and comment on it.
>> However, the Commit-Review can get you *more* reviewers; in addition to
>> those who are interested in the new stuff, you'll find completely random
>> people might speak up, and offer some un-thought-of problem. Even
>> simple things, like a null pointer check happening after a dereference
>> can be found this way.
>

-- 

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Hans Bakker <h....@antwebsystems.com>]

Perhaps also good to know that we are currently implementing:

git-gerrit-jenkins-ofbizscrum of which the git security and peer review  
and ofbizscrum: ticket-backlog-timereg-invoicing is essential to us.

Jira for us is a kind of sideline product, not part of the workflow, 
just sitting on the side line as a manual optional additional step which 
does not automate stuff, just adds extra work.

Regards, Hans


On 14/08/14 08:59, Jacques Le Roux wrote:
>
> Le 14/08/2014 01:24, Adam Heath a écrit :
>> Btw, we(Brainfood) have been using 
>> gitlab(https://about.gitlab.com/gitlab-ce/), which has the MIT 
>> license; I wonder if we could get a copy of that installed, because I 
>> just don't like the commercial github.
>
> You mean at the ASF?
>
> Jacques
>
>>
>> On 08/13/2014 06:17 PM, Adam Heath wrote:
>>> Well, all versions, as it is a new feature.  I developed it directly 
>>> against trunk, not any of a number of older deployed versions.
>>>
>>> I've got another change I've been working on, that I'll do this new 
>>> way.  Basically, Start.java will now take over stdout/stderr, run it 
>>> through a TeeOutputStream, with output being sent do the original 
>>> stdout/stderr, and to a file, then after full startup, the original 
>>> stdout/stderr is turned off. Whenever the output file gets to be a 
>>> certain size, or midnight happens, then the file is auto-rotated, 
>>> and $last_file+1 is auto-compressed.
>>>
>>> I wrote this against trunk as well, in response to java 1.7 running 
>>> on debian wheezy i386, but on top of xen, not working with 2G 
>>> console.log files.  The key difference to this is the "on top of xen".
>>>
>>> Without this patch, ofbiz would need to be restarted externally, and 
>>> the log rotated.  With the patch(es), it stays up basically forever.
>>>
>>> On 08/13/2014 03:21 PM, Jacques Le Roux wrote:
>>>> You can still create a Jira issue and select the concerned versions
>>>>
>>>> Jacques
>>>>
>>>> Le 13/08/2014 21:54, Adam Heath a écrit :
>>>>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>>>>>> As mentioned Jacopo, the Jira issue creation can be done after, 
>>>>>> even by another person.
>>>>>> The point is to collect changes by versions (releases)
>>>>>>
>>>>>> Jacques
>>>>>>
>>>>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>>>>>> Also I forgot to  mention (again) that, with our new policy to 
>>>>>>> automatically grab new features or bug fixes from Jira, a direct 
>>>>>>> commit should not happen for major new features or bug fixes.
>>>>>>> We shall first create a Jira issue, even we you don't submit a 
>>>>>>> patch (which is preferable for peers reviews), in order to fill 
>>>>>>> the version to allow creating reports like
>>>>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel 
>>>>>>>
>>>>>>>
>>>>>
>>>>> Oops, missed this, my bad, on my recent parallelization changes.
>>>>>
>>>>> ps: I really wish that there was some kind of git-based workflow.
>>>>>
>>>>>
>>>>>
>>>
>>
>>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

Le 14/08/2014 01:24, Adam Heath a écrit :
> Btw, we(Brainfood) have been using gitlab(https://about.gitlab.com/gitlab-ce/), which has the MIT license; I wonder if we could get a copy of that 
> installed, because I just don't like the commercial github.

You mean at the ASF?

Jacques

>
> On 08/13/2014 06:17 PM, Adam Heath wrote:
>> Well, all versions, as it is a new feature.  I developed it directly against trunk, not any of a number of older deployed versions.
>>
>> I've got another change I've been working on, that I'll do this new way.  Basically, Start.java will now take over stdout/stderr, run it through a 
>> TeeOutputStream, with output being sent do the original stdout/stderr, and to a file, then after full startup, the original stdout/stderr is turned 
>> off. Whenever the output file gets to be a certain size, or midnight happens, then the file is auto-rotated, and $last_file+1 is auto-compressed.
>>
>> I wrote this against trunk as well, in response to java 1.7 running on debian wheezy i386, but on top of xen, not working with 2G console.log 
>> files.  The key difference to this is the "on top of xen".
>>
>> Without this patch, ofbiz would need to be restarted externally, and the log rotated.  With the patch(es), it stays up basically forever.
>>
>> On 08/13/2014 03:21 PM, Jacques Le Roux wrote:
>>> You can still create a Jira issue and select the concerned versions
>>>
>>> Jacques
>>>
>>> Le 13/08/2014 21:54, Adam Heath a écrit :
>>>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>>>>> As mentioned Jacopo, the Jira issue creation can be done after, even by another person.
>>>>> The point is to collect changes by versions (releases)
>>>>>
>>>>> Jacques
>>>>>
>>>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>>>>> Also I forgot to  mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should 
>>>>>> not happen for major new features or bug fixes.
>>>>>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to 
>>>>>> allow creating reports like
>>>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel
>>>>>>
>>>>
>>>> Oops, missed this, my bad, on my recent parallelization changes.
>>>>
>>>> ps: I really wish that there was some kind of git-based workflow.
>>>>
>>>>
>>>>
>>
>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adam Heath <do...@brainfood.com>]

Btw, we(Brainfood) have been using 
gitlab(https://about.gitlab.com/gitlab-ce/), which has the MIT license; 
I wonder if we could get a copy of that installed, because I just don't 
like the commercial github.

On 08/13/2014 06:17 PM, Adam Heath wrote:
> Well, all versions, as it is a new feature.  I developed it directly 
> against trunk, not any of a number of older deployed versions.
>
> I've got another change I've been working on, that I'll do this new 
> way.  Basically, Start.java will now take over stdout/stderr, run it 
> through a TeeOutputStream, with output being sent do the original 
> stdout/stderr, and to a file, then after full startup, the original 
> stdout/stderr is turned off.  Whenever the output file gets to be a 
> certain size, or midnight happens, then the file is auto-rotated, and 
> $last_file+1 is auto-compressed.
>
> I wrote this against trunk as well, in response to java 1.7 running on 
> debian wheezy i386, but on top of xen, not working with 2G console.log 
> files.  The key difference to this is the "on top of xen".
>
> Without this patch, ofbiz would need to be restarted externally, and 
> the log rotated.  With the patch(es), it stays up basically forever.
>
> On 08/13/2014 03:21 PM, Jacques Le Roux wrote:
>> You can still create a Jira issue and select the concerned versions
>>
>> Jacques
>>
>> Le 13/08/2014 21:54, Adam Heath a écrit :
>>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>>>> As mentioned Jacopo, the Jira issue creation can be done after, 
>>>> even by another person.
>>>> The point is to collect changes by versions (releases)
>>>>
>>>> Jacques
>>>>
>>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>>>> Also I forgot to  mention (again) that, with our new policy to 
>>>>> automatically grab new features or bug fixes from Jira, a direct 
>>>>> commit should not happen for major new features or bug fixes.
>>>>> We shall first create a Jira issue, even we you don't submit a 
>>>>> patch (which is preferable for peers reviews), in order to fill 
>>>>> the version to allow creating reports like
>>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel 
>>>>>
>>>>>
>>>
>>> Oops, missed this, my bad, on my recent parallelization changes.
>>>
>>> ps: I really wish that there was some kind of git-based workflow.
>>>
>>>
>>>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

Le 14/08/2014 01:17, Adam Heath a écrit :
> Well, all versions, as it is a new feature.  I developed it directly against trunk, not any of a number of older deployed versions.

For trunk select rather "Upcoming Branch"

Jacques

>
> I've got another change I've been working on, that I'll do this new way.  Basically, Start.java will now take over stdout/stderr, run it through a 
> TeeOutputStream, with output being sent do the original stdout/stderr, and to a file, then after full startup, the original stdout/stderr is turned 
> off.  Whenever the output file gets to be a certain size, or midnight happens, then the file is auto-rotated, and $last_file+1 is auto-compressed.
>
> I wrote this against trunk as well, in response to java 1.7 running on debian wheezy i386, but on top of xen, not working with 2G console.log 
> files.  The key difference to this is the "on top of xen".
>
> Without this patch, ofbiz would need to be restarted externally, and the log rotated.  With the patch(es), it stays up basically forever.
>
> On 08/13/2014 03:21 PM, Jacques Le Roux wrote:
>> You can still create a Jira issue and select the concerned versions
>>
>> Jacques
>>
>> Le 13/08/2014 21:54, Adam Heath a écrit :
>>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>>>> As mentioned Jacopo, the Jira issue creation can be done after, even by another person.
>>>> The point is to collect changes by versions (releases)
>>>>
>>>> Jacques
>>>>
>>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>>>> Also I forgot to  mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should 
>>>>> not happen for major new features or bug fixes.
>>>>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to 
>>>>> allow creating reports like
>>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel
>>>>>
>>>
>>> Oops, missed this, my bad, on my recent parallelization changes.
>>>
>>> ps: I really wish that there was some kind of git-based workflow.
>>>
>>>
>>>
>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacopo Cappellato <ja...@hotwaxmedia.com>]

On Aug 14, 2014, at 1:17 AM, Adam Heath <do...@brainfood.com> wrote:

> I've got another change I've been working on, that I'll do this new way.  Basically, Start.java will now take over stdout/stderr, [...]

If you are in the process of refactoring Start.java, you could have a look at this newly reported bug:

https://issues.apache.org/jira/browse/OFBIZ-5710

Jacopo

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adam Heath <do...@brainfood.com>]

Well, all versions, as it is a new feature.  I developed it directly 
against trunk, not any of a number of older deployed versions.

I've got another change I've been working on, that I'll do this new 
way.  Basically, Start.java will now take over stdout/stderr, run it 
through a TeeOutputStream, with output being sent do the original 
stdout/stderr, and to a file, then after full startup, the original 
stdout/stderr is turned off.  Whenever the output file gets to be a 
certain size, or midnight happens, then the file is auto-rotated, and 
$last_file+1 is auto-compressed.

I wrote this against trunk as well, in response to java 1.7 running on 
debian wheezy i386, but on top of xen, not working with 2G console.log 
files.  The key difference to this is the "on top of xen".

Without this patch, ofbiz would need to be restarted externally, and the 
log rotated.  With the patch(es), it stays up basically forever.

On 08/13/2014 03:21 PM, Jacques Le Roux wrote:
> You can still create a Jira issue and select the concerned versions
>
> Jacques
>
> Le 13/08/2014 21:54, Adam Heath a écrit :
>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>>> As mentioned Jacopo, the Jira issue creation can be done after, even 
>>> by another person.
>>> The point is to collect changes by versions (releases)
>>>
>>> Jacques
>>>
>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>>> Also I forgot to  mention (again) that, with our new policy to 
>>>> automatically grab new features or bug fixes from Jira, a direct 
>>>> commit should not happen for major new features or bug fixes.
>>>> We shall first create a Jira issue, even we you don't submit a 
>>>> patch (which is preferable for peers reviews), in order to fill the 
>>>> version to allow creating reports like
>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel 
>>>>
>>>>
>>
>> Oops, missed this, my bad, on my recent parallelization changes.
>>
>> ps: I really wish that there was some kind of git-based workflow.
>>
>>
>>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacopo Cappellato <ja...@hotwaxmedia.com>]

Adam,

I have created this Jira ticket for you:

https://issues.apache.org/jira/browse/OFBIZ-5712

with all the fields already set.
It would be great if you could edit the summary/description with the relevant changes you did/are doing and then just resolve it. This will then be automatically included in the release notes for the next branch release.

Thanks,

Jacopo


On Aug 13, 2014, at 10:21 PM, Jacques Le Roux <ja...@les7arts.com> wrote:

> You can still create a Jira issue and select the concerned versions
> 
> Jacques
> 
> Le 13/08/2014 21:54, Adam Heath a écrit :
>> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>>> As mentioned Jacopo, the Jira issue creation can be done after, even by another person.
>>> The point is to collect changes by versions (releases)
>>> 
>>> Jacques
>>> 
>>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>>> Also I forgot to  mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not happen for major new features or bug fixes.
>>>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to allow creating reports like
>>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel
>>>> 
>> 
>> Oops, missed this, my bad, on my recent parallelization changes.
>> 
>> ps: I really wish that there was some kind of git-based workflow.
>> 
>> 
>> 

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

You can still create a Jira issue and select the concerned versions

Jacques

Le 13/08/2014 21:54, Adam Heath a écrit :
> On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
>> As mentioned Jacopo, the Jira issue creation can be done after, even by another person.
>> The point is to collect changes by versions (releases)
>>
>> Jacques
>>
>> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>>> Also I forgot to  mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not 
>>> happen for major new features or bug fixes.
>>> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to 
>>> allow creating reports like
>>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel
>>>
>
> Oops, missed this, my bad, on my recent parallelization changes.
>
> ps: I really wish that there was some kind of git-based workflow.
>
>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adam Heath <do...@brainfood.com>]

On 08/10/2014 08:19 AM, Jacques Le Roux wrote:
> As mentioned Jacopo, the Jira issue creation can be done after, even 
> by another person.
> The point is to collect changes by versions (releases)
>
> Jacques
>
> Le 10/08/2014 14:14, Jacques Le Roux a écrit :
>> Also I forgot to  mention (again) that, with our new policy to 
>> automatically grab new features or bug fixes from Jira, a direct 
>> commit should not happen for major new features or bug fixes.
>> We shall first create a Jira issue, even we you don't submit a patch 
>> (which is preferable for peers reviews), in order to fill the version 
>> to allow creating reports like
>> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel 
>>
>>

Oops, missed this, my bad, on my recent parallelization changes.

ps: I really wish that there was some kind of git-based workflow.

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

As mentioned Jacopo, the Jira issue creation can be done after, even by another person.
The point is to collect changes by versions (releases)

Jacques

Le 10/08/2014 14:14, Jacques Le Roux a écrit :
> Also I forgot to  mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not 
> happen for major new features or bug fixes.
> We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to allow 
> creating reports like
> https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel
>
> Jacques
>
> Le 10/08/2014 12:28, Jacques Le Roux a écrit :
>> I think Sharan would not have included it in her book if it was in R13.07. I guess just a note to say it's not yet ready.
>>
>> There are some guidelines in your 1st mail which could probe useful later, better to see the positive aspects ;)
>>
>> Jacques
>>
>> Le 10/08/2014 00:24, Pierre Smits a écrit :
>>> To clarify: I didn't demand that this code was removed. I advised to
>>> removed it.
>>>
>>> Pierre Smits
>>>
>>> *ORRTIZ.COM <http://www.orrtiz.com>*
>>> Services & Solutions for Cloud-
>>> Based Manufacturing, Professional
>>> Services and Retail & Trade
>>> http://www.orrtiz.com
>>>
>>>
>>> On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum <
>>> adrian.crum@sandglass-software.com> wrote:
>>>
>>>> To clarify: I wasn't trying to push for a particular methodology. I was
>>>> trying to address Pierre's approach to this community, where he demands
>>>> that any code he doesn't like be reverted (he has done this before).
>>>> Traditionally, that is not how we do things in this community.
>>>>
>>>> I agree that a commit that breaks the build process or severely impacts
>>>> the operation of the application should be fixed as quickly as possible or
>>>> it should be reverted. That is not the case in this thread. The new
>>>> functionality might not meet Pierre's standards, but it isn't breaking the
>>>> build. Therefore, I don't agree that it should be reverted.
>>>>
>>>>
>>>> Adrian Crum
>>>> Sandglass Software
>>>> www.sandglass-software.com
>>>>
>>>> On 6/20/2014 9:01 AM, Adam Heath wrote:
>>>>
>>>>> On 06/20/2014 10:42 AM, Adrian Crum wrote:
>>>>>
>>>>>> No one is lowering the bar. The problem is, you still don't understand
>>>>>> how an open source community works.
>>>>>>
>>>>> This is wrong.  There is no hard-set one-workflow-to-bind-them
>>>>> methodology here.  Some people seem to think there is.
>>>>>
>>>>> The Review-Commit workflow people sometimes want all stuff reviewed.
>>>>> While that can help find many issues ahead of time, it will never be
>>>>> able to find and implement all corner cases.  That can generally only
>>>>> happen when the isolated code is pushed into the wider community.  At
>>>>> some point, you just need to push your reviewed code to someone else,
>>>>> and continue to fix/improve afterwards.
>>>>>
>>>>> The Commit-Review workflow can sometimes cause severe breakage for other
>>>>> users.  If not-ready code is pushed into trunk, and it causes feature
>>>>> breakage, or compilation problems, or corruption, the community at large
>>>>> might not be able to help, as the newly pushed code is unknown, and
>>>>> it'll take a while to come up to speed.
>>>>>
>>>>> Speaking about one earlier point: Committing code *early* does not lower
>>>>> the bar for the committed code.  Code is code.  It is, or it isn't. When
>>>>> something is committed has no bearing on the quality of the commit.
>>>>> That quality is a separate item from the time of the commit.
>>>>>
>>>>>   Let me give you an example:
>>>>>> I helped design the custom XML file format OFBiz uses for UI labels
>>>>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people
>>>>>> in the community who disagreed with the design, but it was committed
>>>>>> anyway.
>>>>>>
>>>>> Even here, you didn't do this initial work *in trunk*. You thought
>>>>> about the idea for a while, tried some things, got an initial set, then
>>>>> eventually committed to trunk.
>>>>>
>>>>>   For the next few months, there were a lot of commits to fix bugs that
>>>>>> cropped up after the initial commit. Scott and David helped with the
>>>>>> bug fixes and improvements.
>>>>>>
>>>>>> Eventually, the new feature was working well - but there were still
>>>>>> hundreds of properties files that needed to be converted to the new
>>>>>> format. That was done over a period of several years by many different
>>>>>> people.
>>>>>>
>>>>> Another concrete example.
>>>>>
>>>>> Currently, I'm working on fixes to the entityengine crypt system.
>>>>>
>>>>> 2 years ago, I implemented key-encrypting-key support(read up on the
>>>>> credit card number security docs).  I worked on the code in isolation,
>>>>> on behalf of a customer.  Once it worked there, I then directly pushed
>>>>> to ofbiz trunk.  This is the Commit-Review workflow.
>>>>>
>>>>> No review happened.  None.  If such review had happened, it might have
>>>>> discovered that direct lookup of encrypted fields would have been broken
>>>>> by my new code(a random salt is prepended to each encrypted value, which
>>>>> prevents lookups).
>>>>>
>>>>> Even if that review *had* happened, after the commit, or even *before*
>>>>> the commit, and I didn't add that random salt, it *still* wouldn't have
>>>>> fixed the direct lookup problem.  This was due to direct-lookup being
>>>>> broken as far back as release4.0, and probably even all the way back to
>>>>> 2005(!).  This points to a general lack of review, at *all* levels.
>>>>>
>>>>> It's been my experience, that the Review-Commit workflow will get you
>>>>> some small about of early reviews; those people who are keenly
>>>>> interested in your new idea will possibly wake up and comment on it.
>>>>> However, the Commit-Review can get you *more* reviewers; in addition to
>>>>> those who are interested in the new stuff, you'll find completely random
>>>>> people might speak up, and offer some un-thought-of problem.  Even
>>>>> simple things, like a null pointer check happening after a dereference
>>>>> can be found this way.
>>>>>
>>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

Also I forgot to  mention (again) that, with our new policy to automatically grab new features or bug fixes from Jira, a direct commit should not 
happen for major new features or bug fixes.
We shall first create a Jira issue, even we you don't submit a patch (which is preferable for peers reviews), in order to fill the version to allow 
creating reports like
https://issues.apache.org/jira/browse/OFBIZ/fixforversion/12327361/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel

Jacques

Le 10/08/2014 12:28, Jacques Le Roux a écrit :
> I think Sharan would not have included it in her book if it was in R13.07. I guess just a note to say it's not yet ready.
>
> There are some guidelines in your 1st mail which could probe useful later, better to see the positive aspects ;)
>
> Jacques
>
> Le 10/08/2014 00:24, Pierre Smits a écrit :
>> To clarify: I didn't demand that this code was removed. I advised to
>> removed it.
>>
>> Pierre Smits
>>
>> *ORRTIZ.COM <http://www.orrtiz.com>*
>> Services & Solutions for Cloud-
>> Based Manufacturing, Professional
>> Services and Retail & Trade
>> http://www.orrtiz.com
>>
>>
>> On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum <
>> adrian.crum@sandglass-software.com> wrote:
>>
>>> To clarify: I wasn't trying to push for a particular methodology. I was
>>> trying to address Pierre's approach to this community, where he demands
>>> that any code he doesn't like be reverted (he has done this before).
>>> Traditionally, that is not how we do things in this community.
>>>
>>> I agree that a commit that breaks the build process or severely impacts
>>> the operation of the application should be fixed as quickly as possible or
>>> it should be reverted. That is not the case in this thread. The new
>>> functionality might not meet Pierre's standards, but it isn't breaking the
>>> build. Therefore, I don't agree that it should be reverted.
>>>
>>>
>>> Adrian Crum
>>> Sandglass Software
>>> www.sandglass-software.com
>>>
>>> On 6/20/2014 9:01 AM, Adam Heath wrote:
>>>
>>>> On 06/20/2014 10:42 AM, Adrian Crum wrote:
>>>>
>>>>> No one is lowering the bar. The problem is, you still don't understand
>>>>> how an open source community works.
>>>>>
>>>> This is wrong.  There is no hard-set one-workflow-to-bind-them
>>>> methodology here.  Some people seem to think there is.
>>>>
>>>> The Review-Commit workflow people sometimes want all stuff reviewed.
>>>> While that can help find many issues ahead of time, it will never be
>>>> able to find and implement all corner cases.  That can generally only
>>>> happen when the isolated code is pushed into the wider community.  At
>>>> some point, you just need to push your reviewed code to someone else,
>>>> and continue to fix/improve afterwards.
>>>>
>>>> The Commit-Review workflow can sometimes cause severe breakage for other
>>>> users.  If not-ready code is pushed into trunk, and it causes feature
>>>> breakage, or compilation problems, or corruption, the community at large
>>>> might not be able to help, as the newly pushed code is unknown, and
>>>> it'll take a while to come up to speed.
>>>>
>>>> Speaking about one earlier point: Committing code *early* does not lower
>>>> the bar for the committed code.  Code is code.  It is, or it isn't. When
>>>> something is committed has no bearing on the quality of the commit.
>>>> That quality is a separate item from the time of the commit.
>>>>
>>>>   Let me give you an example:
>>>>> I helped design the custom XML file format OFBiz uses for UI labels
>>>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people
>>>>> in the community who disagreed with the design, but it was committed
>>>>> anyway.
>>>>>
>>>> Even here, you didn't do this initial work *in trunk*.  You thought
>>>> about the idea for a while, tried some things, got an initial set, then
>>>> eventually committed to trunk.
>>>>
>>>>   For the next few months, there were a lot of commits to fix bugs that
>>>>> cropped up after the initial commit. Scott and David helped with the
>>>>> bug fixes and improvements.
>>>>>
>>>>> Eventually, the new feature was working well - but there were still
>>>>> hundreds of properties files that needed to be converted to the new
>>>>> format. That was done over a period of several years by many different
>>>>> people.
>>>>>
>>>> Another concrete example.
>>>>
>>>> Currently, I'm working on fixes to the entityengine crypt system.
>>>>
>>>> 2 years ago, I implemented key-encrypting-key support(read up on the
>>>> credit card number security docs).  I worked on the code in isolation,
>>>> on behalf of a customer.  Once it worked there, I then directly pushed
>>>> to ofbiz trunk.  This is the Commit-Review workflow.
>>>>
>>>> No review happened.  None.  If such review had happened, it might have
>>>> discovered that direct lookup of encrypted fields would have been broken
>>>> by my new code(a random salt is prepended to each encrypted value, which
>>>> prevents lookups).
>>>>
>>>> Even if that review *had* happened, after the commit, or even *before*
>>>> the commit, and I didn't add that random salt, it *still* wouldn't have
>>>> fixed the direct lookup problem.  This was due to direct-lookup being
>>>> broken as far back as release4.0, and probably even all the way back to
>>>> 2005(!).  This points to a general lack of review, at *all* levels.
>>>>
>>>> It's been my experience, that the Review-Commit workflow will get you
>>>> some small about of early reviews; those people who are keenly
>>>> interested in your new idea will possibly wake up and comment on it.
>>>> However, the Commit-Review can get you *more* reviewers; in addition to
>>>> those who are interested in the new stuff, you'll find completely random
>>>> people might speak up, and offer some un-thought-of problem.  Even
>>>> simple things, like a null pointer check happening after a dereference
>>>> can be found this way.
>>>>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Jacques Le Roux <ja...@les7arts.com>]

I think Sharan would not have included it in her book if it was in R13.07. I guess just a note to say it's not yet ready.

There are some guidelines in your 1st mail which could probe useful later, better to see the positive aspects ;)

Jacques

Le 10/08/2014 00:24, Pierre Smits a écrit :
> To clarify: I didn't demand that this code was removed. I advised to
> removed it.
>
> Pierre Smits
>
> *ORRTIZ.COM <http://www.orrtiz.com>*
> Services & Solutions for Cloud-
> Based Manufacturing, Professional
> Services and Retail & Trade
> http://www.orrtiz.com
>
>
> On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum <
> adrian.crum@sandglass-software.com> wrote:
>
>> To clarify: I wasn't trying to push for a particular methodology. I was
>> trying to address Pierre's approach to this community, where he demands
>> that any code he doesn't like be reverted (he has done this before).
>> Traditionally, that is not how we do things in this community.
>>
>> I agree that a commit that breaks the build process or severely impacts
>> the operation of the application should be fixed as quickly as possible or
>> it should be reverted. That is not the case in this thread. The new
>> functionality might not meet Pierre's standards, but it isn't breaking the
>> build. Therefore, I don't agree that it should be reverted.
>>
>>
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>>
>> On 6/20/2014 9:01 AM, Adam Heath wrote:
>>
>>> On 06/20/2014 10:42 AM, Adrian Crum wrote:
>>>
>>>> No one is lowering the bar. The problem is, you still don't understand
>>>> how an open source community works.
>>>>
>>> This is wrong.  There is no hard-set one-workflow-to-bind-them
>>> methodology here.  Some people seem to think there is.
>>>
>>> The Review-Commit workflow people sometimes want all stuff reviewed.
>>> While that can help find many issues ahead of time, it will never be
>>> able to find and implement all corner cases.  That can generally only
>>> happen when the isolated code is pushed into the wider community.  At
>>> some point, you just need to push your reviewed code to someone else,
>>> and continue to fix/improve afterwards.
>>>
>>> The Commit-Review workflow can sometimes cause severe breakage for other
>>> users.  If not-ready code is pushed into trunk, and it causes feature
>>> breakage, or compilation problems, or corruption, the community at large
>>> might not be able to help, as the newly pushed code is unknown, and
>>> it'll take a while to come up to speed.
>>>
>>> Speaking about one earlier point: Committing code *early* does not lower
>>> the bar for the committed code.  Code is code.  It is, or it isn't. When
>>> something is committed has no bearing on the quality of the commit.
>>> That quality is a separate item from the time of the commit.
>>>
>>>   Let me give you an example:
>>>> I helped design the custom XML file format OFBiz uses for UI labels
>>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people
>>>> in the community who disagreed with the design, but it was committed
>>>> anyway.
>>>>
>>> Even here, you didn't do this initial work *in trunk*.  You thought
>>> about the idea for a while, tried some things, got an initial set, then
>>> eventually committed to trunk.
>>>
>>>   For the next few months, there were a lot of commits to fix bugs that
>>>> cropped up after the initial commit. Scott and David helped with the
>>>> bug fixes and improvements.
>>>>
>>>> Eventually, the new feature was working well - but there were still
>>>> hundreds of properties files that needed to be converted to the new
>>>> format. That was done over a period of several years by many different
>>>> people.
>>>>
>>> Another concrete example.
>>>
>>> Currently, I'm working on fixes to the entityengine crypt system.
>>>
>>> 2 years ago, I implemented key-encrypting-key support(read up on the
>>> credit card number security docs).  I worked on the code in isolation,
>>> on behalf of a customer.  Once it worked there, I then directly pushed
>>> to ofbiz trunk.  This is the Commit-Review workflow.
>>>
>>> No review happened.  None.  If such review had happened, it might have
>>> discovered that direct lookup of encrypted fields would have been broken
>>> by my new code(a random salt is prepended to each encrypted value, which
>>> prevents lookups).
>>>
>>> Even if that review *had* happened, after the commit, or even *before*
>>> the commit, and I didn't add that random salt, it *still* wouldn't have
>>> fixed the direct lookup problem.  This was due to direct-lookup being
>>> broken as far back as release4.0, and probably even all the way back to
>>> 2005(!).  This points to a general lack of review, at *all* levels.
>>>
>>> It's been my experience, that the Review-Commit workflow will get you
>>> some small about of early reviews; those people who are keenly
>>> interested in your new idea will possibly wake up and comment on it.
>>> However, the Commit-Review can get you *more* reviewers; in addition to
>>> those who are interested in the new stuff, you'll find completely random
>>> people might speak up, and offer some un-thought-of problem.  Even
>>> simple things, like a null pointer check happening after a dereference
>>> can be found this way.
>>>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Pierre Smits <pi...@gmail.com>]

To clarify: I didn't demand that this code was removed. I advised to
removed it.

Pierre Smits

*ORRTIZ.COM <http://www.orrtiz.com>*
Services & Solutions for Cloud-
Based Manufacturing, Professional
Services and Retail & Trade
http://www.orrtiz.com


On Fri, Jun 20, 2014 at 6:32 PM, Adrian Crum <
adrian.crum@sandglass-software.com> wrote:

> To clarify: I wasn't trying to push for a particular methodology. I was
> trying to address Pierre's approach to this community, where he demands
> that any code he doesn't like be reverted (he has done this before).
> Traditionally, that is not how we do things in this community.
>
> I agree that a commit that breaks the build process or severely impacts
> the operation of the application should be fixed as quickly as possible or
> it should be reverted. That is not the case in this thread. The new
> functionality might not meet Pierre's standards, but it isn't breaking the
> build. Therefore, I don't agree that it should be reverted.
>
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
> On 6/20/2014 9:01 AM, Adam Heath wrote:
>
>> On 06/20/2014 10:42 AM, Adrian Crum wrote:
>>
>>> No one is lowering the bar. The problem is, you still don't understand
>>> how an open source community works.
>>>
>>
>> This is wrong.  There is no hard-set one-workflow-to-bind-them
>> methodology here.  Some people seem to think there is.
>>
>> The Review-Commit workflow people sometimes want all stuff reviewed.
>> While that can help find many issues ahead of time, it will never be
>> able to find and implement all corner cases.  That can generally only
>> happen when the isolated code is pushed into the wider community.  At
>> some point, you just need to push your reviewed code to someone else,
>> and continue to fix/improve afterwards.
>>
>> The Commit-Review workflow can sometimes cause severe breakage for other
>> users.  If not-ready code is pushed into trunk, and it causes feature
>> breakage, or compilation problems, or corruption, the community at large
>> might not be able to help, as the newly pushed code is unknown, and
>> it'll take a while to come up to speed.
>>
>> Speaking about one earlier point: Committing code *early* does not lower
>> the bar for the committed code.  Code is code.  It is, or it isn't. When
>> something is committed has no bearing on the quality of the commit.
>> That quality is a separate item from the time of the commit.
>>
>>  Let me give you an example:
>>>
>>> I helped design the custom XML file format OFBiz uses for UI labels
>>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people
>>> in the community who disagreed with the design, but it was committed
>>> anyway.
>>>
>>
>> Even here, you didn't do this initial work *in trunk*.  You thought
>> about the idea for a while, tried some things, got an initial set, then
>> eventually committed to trunk.
>>
>>  For the next few months, there were a lot of commits to fix bugs that
>>> cropped up after the initial commit. Scott and David helped with the
>>> bug fixes and improvements.
>>>
>>> Eventually, the new feature was working well - but there were still
>>> hundreds of properties files that needed to be converted to the new
>>> format. That was done over a period of several years by many different
>>> people.
>>>
>>
>> Another concrete example.
>>
>> Currently, I'm working on fixes to the entityengine crypt system.
>>
>> 2 years ago, I implemented key-encrypting-key support(read up on the
>> credit card number security docs).  I worked on the code in isolation,
>> on behalf of a customer.  Once it worked there, I then directly pushed
>> to ofbiz trunk.  This is the Commit-Review workflow.
>>
>> No review happened.  None.  If such review had happened, it might have
>> discovered that direct lookup of encrypted fields would have been broken
>> by my new code(a random salt is prepended to each encrypted value, which
>> prevents lookups).
>>
>> Even if that review *had* happened, after the commit, or even *before*
>> the commit, and I didn't add that random salt, it *still* wouldn't have
>> fixed the direct lookup problem.  This was due to direct-lookup being
>> broken as far back as release4.0, and probably even all the way back to
>> 2005(!).  This points to a general lack of review, at *all* levels.
>>
>> It's been my experience, that the Review-Commit workflow will get you
>> some small about of early reviews; those people who are keenly
>> interested in your new idea will possibly wake up and comment on it.
>> However, the Commit-Review can get you *more* reviewers; in addition to
>> those who are interested in the new stuff, you'll find completely random
>> people might speak up, and offer some un-thought-of problem.  Even
>> simple things, like a null pointer check happening after a dereference
>> can be found this way.
>>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adrian Crum <ad...@sandglass-software.com>]

To clarify: I wasn't trying to push for a particular methodology. I was 
trying to address Pierre's approach to this community, where he demands 
that any code he doesn't like be reverted (he has done this before). 
Traditionally, that is not how we do things in this community.

I agree that a commit that breaks the build process or severely impacts 
the operation of the application should be fixed as quickly as possible 
or it should be reverted. That is not the case in this thread. The new 
functionality might not meet Pierre's standards, but it isn't breaking 
the build. Therefore, I don't agree that it should be reverted.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 6/20/2014 9:01 AM, Adam Heath wrote:
> On 06/20/2014 10:42 AM, Adrian Crum wrote:
>> No one is lowering the bar. The problem is, you still don't understand
>> how an open source community works.
>
> This is wrong.  There is no hard-set one-workflow-to-bind-them
> methodology here.  Some people seem to think there is.
>
> The Review-Commit workflow people sometimes want all stuff reviewed.
> While that can help find many issues ahead of time, it will never be
> able to find and implement all corner cases.  That can generally only
> happen when the isolated code is pushed into the wider community.  At
> some point, you just need to push your reviewed code to someone else,
> and continue to fix/improve afterwards.
>
> The Commit-Review workflow can sometimes cause severe breakage for other
> users.  If not-ready code is pushed into trunk, and it causes feature
> breakage, or compilation problems, or corruption, the community at large
> might not be able to help, as the newly pushed code is unknown, and
> it'll take a while to come up to speed.
>
> Speaking about one earlier point: Committing code *early* does not lower
> the bar for the committed code.  Code is code.  It is, or it isn't. When
> something is committed has no bearing on the quality of the commit.
> That quality is a separate item from the time of the commit.
>
>> Let me give you an example:
>>
>> I helped design the custom XML file format OFBiz uses for UI labels
>> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people
>> in the community who disagreed with the design, but it was committed
>> anyway.
>
> Even here, you didn't do this initial work *in trunk*.  You thought
> about the idea for a while, tried some things, got an initial set, then
> eventually committed to trunk.
>
>> For the next few months, there were a lot of commits to fix bugs that
>> cropped up after the initial commit. Scott and David helped with the
>> bug fixes and improvements.
>>
>> Eventually, the new feature was working well - but there were still
>> hundreds of properties files that needed to be converted to the new
>> format. That was done over a period of several years by many different
>> people.
>
> Another concrete example.
>
> Currently, I'm working on fixes to the entityengine crypt system.
>
> 2 years ago, I implemented key-encrypting-key support(read up on the
> credit card number security docs).  I worked on the code in isolation,
> on behalf of a customer.  Once it worked there, I then directly pushed
> to ofbiz trunk.  This is the Commit-Review workflow.
>
> No review happened.  None.  If such review had happened, it might have
> discovered that direct lookup of encrypted fields would have been broken
> by my new code(a random salt is prepended to each encrypted value, which
> prevents lookups).
>
> Even if that review *had* happened, after the commit, or even *before*
> the commit, and I didn't add that random salt, it *still* wouldn't have
> fixed the direct lookup problem.  This was due to direct-lookup being
> broken as far back as release4.0, and probably even all the way back to
> 2005(!).  This points to a general lack of review, at *all* levels.
>
> It's been my experience, that the Review-Commit workflow will get you
> some small about of early reviews; those people who are keenly
> interested in your new idea will possibly wake up and comment on it.
> However, the Commit-Review can get you *more* reviewers; in addition to
> those who are interested in the new stuff, you'll find completely random
> people might speak up, and offer some un-thought-of problem.  Even
> simple things, like a null pointer check happening after a dereference
> can be found this way.

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adam Heath <do...@brainfood.com>]

On 06/20/2014 10:42 AM, Adrian Crum wrote:
> No one is lowering the bar. The problem is, you still don't understand 
> how an open source community works.

This is wrong.  There is no hard-set one-workflow-to-bind-them 
methodology here.  Some people seem to think there is.

The Review-Commit workflow people sometimes want all stuff reviewed.  
While that can help find many issues ahead of time, it will never be 
able to find and implement all corner cases.  That can generally only 
happen when the isolated code is pushed into the wider community.  At 
some point, you just need to push your reviewed code to someone else, 
and continue to fix/improve afterwards.

The Commit-Review workflow can sometimes cause severe breakage for other 
users.  If not-ready code is pushed into trunk, and it causes feature 
breakage, or compilation problems, or corruption, the community at large 
might not be able to help, as the newly pushed code is unknown, and 
it'll take a while to come up to speed.

Speaking about one earlier point: Committing code *early* does not lower 
the bar for the committed code.  Code is code.  It is, or it isn't.  
When something is committed has no bearing on the quality of the 
commit.  That quality is a separate item from the time of the commit.

> Let me give you an example:
>
> I helped design the custom XML file format OFBiz uses for UI labels 
> (https://issues.apache.org/jira/browse/OFBIZ-1442). There were people 
> in the community who disagreed with the design, but it was committed 
> anyway.

Even here, you didn't do this initial work *in trunk*.  You thought 
about the idea for a while, tried some things, got an initial set, then 
eventually committed to trunk.

> For the next few months, there were a lot of commits to fix bugs that 
> cropped up after the initial commit. Scott and David helped with the 
> bug fixes and improvements.
>
> Eventually, the new feature was working well - but there were still 
> hundreds of properties files that needed to be converted to the new 
> format. That was done over a period of several years by many different 
> people.

Another concrete example.

Currently, I'm working on fixes to the entityengine crypt system.

2 years ago, I implemented key-encrypting-key support(read up on the 
credit card number security docs).  I worked on the code in isolation, 
on behalf of a customer.  Once it worked there, I then directly pushed 
to ofbiz trunk.  This is the Commit-Review workflow.

No review happened.  None.  If such review had happened, it might have 
discovered that direct lookup of encrypted fields would have been broken 
by my new code(a random salt is prepended to each encrypted value, which 
prevents lookups).

Even if that review *had* happened, after the commit, or even *before* 
the commit, and I didn't add that random salt, it *still* wouldn't have 
fixed the direct lookup problem.  This was due to direct-lookup being 
broken as far back as release4.0, and probably even all the way back to 
2005(!).  This points to a general lack of review, at *all* levels.

It's been my experience, that the Review-Commit workflow will get you 
some small about of early reviews; those people who are keenly 
interested in your new idea will possibly wake up and comment on it.  
However, the Commit-Review can get you *more* reviewers; in addition to 
those who are interested in the new stuff, you'll find completely random 
people might speak up, and offer some un-thought-of problem.  Even 
simple things, like a null pointer check happening after a dereference 
can be found this way.

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adrian Crum <ad...@sandglass-software.com>]

No one is lowering the bar. The problem is, you still don't understand 
how an open source community works.

Let me give you an example:

I helped design the custom XML file format OFBiz uses for UI labels 
(https://issues.apache.org/jira/browse/OFBIZ-1442). There were people in 
the community who disagreed with the design, but it was committed anyway.

For the next few months, there were a lot of commits to fix bugs that 
cropped up after the initial commit. Scott and David helped with the bug 
fixes and improvements.

Eventually, the new feature was working well - but there were still 
hundreds of properties files that needed to be converted to the new 
format. That was done over a period of several years by many different 
people.

Today, it would be hard to imagine OFBiz without the custom XML format.

This is how open source works. We start with a bit of code and then 
everyone contributes their part to improve it. If we did things your way 
- code is not committed until it is perfectly complete and bug free - 
then the project would come to a screeching halt.

You have a choice:

1. Continue trying to force everyone in the community to accept your 
viewpoint of how the community should work.

2. Accept how the community works and participate in it.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 6/19/2014 11:54 PM, Pierre Smits wrote:
> Adrian,
>
> Thank you for your opinion. And for lowering the bar.
>
> This code dump isn't a workable solution for - as a caption on the OFBiz
> website states - 'the best e-commerce and Enterprise Resource Planning
> (ERP) software available.
>
> Instead it is a waste of time and an insult to everybody who designs and
> develops good, thought-through business solutions for OFBiz, and you should
> be offended to.
>
> Did Hans hand over his committers userId and password to one of his junior
> developers or the intern of the week? Because this is definitely not even
> close to the same level of quality and coherence as the project mgt or
> scrum solution. And if he didn't, it can surely not be regarded as 'going
> the extra mile' visavis completeness or working with the community.
>
>
>
> Pierre Smits
>
> *ORRTIZ.COM <http://www.orrtiz.com>*
> Services & Solutions for Cloud-
> Based Manufacturing, Professional
> Services and Retail & Trade
> http://www.orrtiz.com
>
>
> On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum <
> adrian.crum@sandglass-software.com> wrote:
>
>> I don't see why it needs to be reverted. If it needs to be built out more,
>> then devs can supply patches for that.
>>
>> There have been many times in the past where some basic functionality is
>> introduced to the project, and then it is up to the rest of the community
>> to finish building it out.
>>
>> Adrian Crum
>> Sandglass Software
>> www.sandglass-software.com
>>
>>
>> On 6/19/2014 5:26 AM, Pierre Smits wrote:
>>
>>> Hi All,
>>>
>>> I finally had some time to review the functionality and code submitted
>>> under:
>>>
>>>      - http://svn.apache.org/r1573884
>>>      - http://svn.apache.org/r1574400
>>>
>>>
>>> The functionality is available in the demo environment on
>>> https://ofbiz-vm.apache.org:8443/accounting
>>>
>>> The code is related to OFBIZ-3169
>>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the
>>>
>>> committer (Hans Bakker) didn't feel the need to follow due process by
>>> adding the improvement as a patch for review by other community members
>>> before dumping it into trunk.
>>> Though it may seem that missing budget functionality can be regarded as a
>>> bug, it should be considered an improvement and therefore it should follow
>>> the Review-then-Commit principle in stead of Commit-then-Review.
>>>
>>> Functionality overview:
>>> Basically the functionality allows users of the the accounting component
>>> to:
>>>
>>>      - search and find existing budget request
>>>      - create a new budget request
>>>      - enhance the budget request with
>>>         - budget items
>>>         - budget request roles
>>>         - budget request reviews
>>>      - change the status of the budget request
>>>
>>>
>>> Findings:
>>> First of all, documentation describing the functionality and how to use it
>>> is not available. This is a mayor omission. Assessment of usability by end
>>> users is hindered by this lacking.
>>>
>>> Secondly, the functionality as presented works. No errors occurred when
>>> clicking on the various buttons and executing basic process steps, like:
>>>
>>>      - adding removing budget items
>>>      - adding and removing a review
>>>      - adding and removing a role
>>>      - Changing the status of the budget request
>>>
>>>
>>> But, can we say that this functionality is complete and lifts OFBiz in
>>> general and the accounting module in particular a step higher on the
>>> ladder
>>> towards best of breed in the category 'Open Source ERP Projects c.q.
>>> Solutions'?
>>>
>>> It definitely does not. The functionality appears, though included in the
>>> accounting module, to be an island. Budgets in general (and at least), are
>>> related to organisations (in OFBiz the internal organisations), P&L
>>> centers, gl accounts, and persons. This is lacking in the functionality.
>>> No
>>> internal organisation can be assigned, nor a P&L center or at the lowest a
>>> gl account. Budgets without such associations are meaningless.
>>>
>>> No permissions where incorporated in menu-items to ensure that
>>> functionalities can only be executed by appropriate users. No roles were
>>> predefined, which is customarily applicable regarding this kind of
>>> functionality. Not every users having access should be able to create
>>> budget request, or perform subsequent actions.
>>>
>>> Lacking this, the user can (must?) select any partyId and roleId to
>>> associate with the budget request. Even parties not working with (or
>>> allowed to work with) OFBiz in general and the accounting module in
>>> particular can be selected.
>>> But also due to the lacking possibility to set the internal organisation,
>>> no default currency is set. Thus, making the amounts ambiguous to
>>> interpret.
>>>
>>> Furthermore it provides only a basic workflow process consisting of
>>> (approve, reject and review). Associating a party to a budget request
>>> doesn't warrant that others can not manipulate the content/data associated
>>> with the request. When I approved a budget request, I had the button to
>>> reject available. This shouldn't be possible. And at one occasion I could
>>> click the 'reject' button multipe time to get to the final state.
>>> While status changes are registered, it should also show who invoked the
>>> status change.
>>>
>>> Due to the fact that arbitrarily any party can be associated to a budget
>>> request without any restrictions, the workflow is rendered meaningless.
>>>
>>> Conclusion(s):
>>> This solutions hasn't been thought through properly and does no good for
>>> the quality of OFBiz (both the project and the product) in general and for
>>> the accounting module in particular. Prior to dumping the code into trunk
>>> it should have been posted for review by the community, so this could have
>>> been avoided.
>>>
>>> If any other contributor to this project made such a set of
>>> functionalities
>>> available as a patch to an OFBiz issue in JIRA, it would sit there for a
>>> great length of time before it would have been committed.
>>>
>>> I advice to remove it from trunk.
>>>
>>> Regards,
>>>
>>> Pierre Smits
>>>
>>> *ORRTIZ.COM <http://www.orrtiz.com>*
>>>
>>> Services & Solutions for Cloud-
>>> Based Manufacturing, Professional
>>> Services and Retail & Trade
>>> http://www.orrtiz.com
>>>
>>>
>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Pierre Smits <pi...@gmail.com>]

Adrian,

Thank you for your opinion. And for lowering the bar.

This code dump isn't a workable solution for - as a caption on the OFBiz
website states - 'the best e-commerce and Enterprise Resource Planning
(ERP) software available.

Instead it is a waste of time and an insult to everybody who designs and
develops good, thought-through business solutions for OFBiz, and you should
be offended to.

Did Hans hand over his committers userId and password to one of his junior
developers or the intern of the week? Because this is definitely not even
close to the same level of quality and coherence as the project mgt or
scrum solution. And if he didn't, it can surely not be regarded as 'going
the extra mile' visavis completeness or working with the community.



Pierre Smits

*ORRTIZ.COM <http://www.orrtiz.com>*
Services & Solutions for Cloud-
Based Manufacturing, Professional
Services and Retail & Trade
http://www.orrtiz.com


On Thu, Jun 19, 2014 at 4:19 PM, Adrian Crum <
adrian.crum@sandglass-software.com> wrote:

> I don't see why it needs to be reverted. If it needs to be built out more,
> then devs can supply patches for that.
>
> There have been many times in the past where some basic functionality is
> introduced to the project, and then it is up to the rest of the community
> to finish building it out.
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
>
> On 6/19/2014 5:26 AM, Pierre Smits wrote:
>
>> Hi All,
>>
>> I finally had some time to review the functionality and code submitted
>> under:
>>
>>     - http://svn.apache.org/r1573884
>>     - http://svn.apache.org/r1574400
>>
>>
>> The functionality is available in the demo environment on
>> https://ofbiz-vm.apache.org:8443/accounting
>>
>> The code is related to OFBIZ-3169
>> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the
>>
>> committer (Hans Bakker) didn't feel the need to follow due process by
>> adding the improvement as a patch for review by other community members
>> before dumping it into trunk.
>> Though it may seem that missing budget functionality can be regarded as a
>> bug, it should be considered an improvement and therefore it should follow
>> the Review-then-Commit principle in stead of Commit-then-Review.
>>
>> Functionality overview:
>> Basically the functionality allows users of the the accounting component
>> to:
>>
>>     - search and find existing budget request
>>     - create a new budget request
>>     - enhance the budget request with
>>        - budget items
>>        - budget request roles
>>        - budget request reviews
>>     - change the status of the budget request
>>
>>
>> Findings:
>> First of all, documentation describing the functionality and how to use it
>> is not available. This is a mayor omission. Assessment of usability by end
>> users is hindered by this lacking.
>>
>> Secondly, the functionality as presented works. No errors occurred when
>> clicking on the various buttons and executing basic process steps, like:
>>
>>     - adding removing budget items
>>     - adding and removing a review
>>     - adding and removing a role
>>     - Changing the status of the budget request
>>
>>
>> But, can we say that this functionality is complete and lifts OFBiz in
>> general and the accounting module in particular a step higher on the
>> ladder
>> towards best of breed in the category 'Open Source ERP Projects c.q.
>> Solutions'?
>>
>> It definitely does not. The functionality appears, though included in the
>> accounting module, to be an island. Budgets in general (and at least), are
>> related to organisations (in OFBiz the internal organisations), P&L
>> centers, gl accounts, and persons. This is lacking in the functionality.
>> No
>> internal organisation can be assigned, nor a P&L center or at the lowest a
>> gl account. Budgets without such associations are meaningless.
>>
>> No permissions where incorporated in menu-items to ensure that
>> functionalities can only be executed by appropriate users. No roles were
>> predefined, which is customarily applicable regarding this kind of
>> functionality. Not every users having access should be able to create
>> budget request, or perform subsequent actions.
>>
>> Lacking this, the user can (must?) select any partyId and roleId to
>> associate with the budget request. Even parties not working with (or
>> allowed to work with) OFBiz in general and the accounting module in
>> particular can be selected.
>> But also due to the lacking possibility to set the internal organisation,
>> no default currency is set. Thus, making the amounts ambiguous to
>> interpret.
>>
>> Furthermore it provides only a basic workflow process consisting of
>> (approve, reject and review). Associating a party to a budget request
>> doesn't warrant that others can not manipulate the content/data associated
>> with the request. When I approved a budget request, I had the button to
>> reject available. This shouldn't be possible. And at one occasion I could
>> click the 'reject' button multipe time to get to the final state.
>> While status changes are registered, it should also show who invoked the
>> status change.
>>
>> Due to the fact that arbitrarily any party can be associated to a budget
>> request without any restrictions, the workflow is rendered meaningless.
>>
>> Conclusion(s):
>> This solutions hasn't been thought through properly and does no good for
>> the quality of OFBiz (both the project and the product) in general and for
>> the accounting module in particular. Prior to dumping the code into trunk
>> it should have been posted for review by the community, so this could have
>> been avoided.
>>
>> If any other contributor to this project made such a set of
>> functionalities
>> available as a patch to an OFBiz issue in JIRA, it would sit there for a
>> great length of time before it would have been committed.
>>
>> I advice to remove it from trunk.
>>
>> Regards,
>>
>> Pierre Smits
>>
>> *ORRTIZ.COM <http://www.orrtiz.com>*
>>
>> Services & Solutions for Cloud-
>> Based Manufacturing, Professional
>> Services and Retail & Trade
>> http://www.orrtiz.com
>>
>>

Re: Review of Accounting/Budget functionality via r1573884 & r1574400

[Adrian Crum <ad...@sandglass-software.com>]

I don't see why it needs to be reverted. If it needs to be built out 
more, then devs can supply patches for that.

There have been many times in the past where some basic functionality is 
introduced to the project, and then it is up to the rest of the 
community to finish building it out.

Adrian Crum
Sandglass Software
www.sandglass-software.com

On 6/19/2014 5:26 AM, Pierre Smits wrote:
> Hi All,
>
> I finally had some time to review the functionality and code submitted
> under:
>
>     - http://svn.apache.org/r1573884
>     - http://svn.apache.org/r1574400
>
> The functionality is available in the demo environment on
> https://ofbiz-vm.apache.org:8443/accounting
>
> The code is related to OFBIZ-3169
> <https://issues.apache.org/jira/browse/OFBIZ-3169>, but apparently the
> committer (Hans Bakker) didn't feel the need to follow due process by
> adding the improvement as a patch for review by other community members
> before dumping it into trunk.
> Though it may seem that missing budget functionality can be regarded as a
> bug, it should be considered an improvement and therefore it should follow
> the Review-then-Commit principle in stead of Commit-then-Review.
>
> Functionality overview:
> Basically the functionality allows users of the the accounting component to:
>
>     - search and find existing budget request
>     - create a new budget request
>     - enhance the budget request with
>        - budget items
>        - budget request roles
>        - budget request reviews
>     - change the status of the budget request
>
> Findings:
> First of all, documentation describing the functionality and how to use it
> is not available. This is a mayor omission. Assessment of usability by end
> users is hindered by this lacking.
>
> Secondly, the functionality as presented works. No errors occurred when
> clicking on the various buttons and executing basic process steps, like:
>
>     - adding removing budget items
>     - adding and removing a review
>     - adding and removing a role
>     - Changing the status of the budget request
>
> But, can we say that this functionality is complete and lifts OFBiz in
> general and the accounting module in particular a step higher on the ladder
> towards best of breed in the category 'Open Source ERP Projects c.q.
> Solutions'?
>
> It definitely does not. The functionality appears, though included in the
> accounting module, to be an island. Budgets in general (and at least), are
> related to organisations (in OFBiz the internal organisations), P&L
> centers, gl accounts, and persons. This is lacking in the functionality. No
> internal organisation can be assigned, nor a P&L center or at the lowest a
> gl account. Budgets without such associations are meaningless.
>
> No permissions where incorporated in menu-items to ensure that
> functionalities can only be executed by appropriate users. No roles were
> predefined, which is customarily applicable regarding this kind of
> functionality. Not every users having access should be able to create
> budget request, or perform subsequent actions.
>
> Lacking this, the user can (must?) select any partyId and roleId to
> associate with the budget request. Even parties not working with (or
> allowed to work with) OFBiz in general and the accounting module in
> particular can be selected.
> But also due to the lacking possibility to set the internal organisation,
> no default currency is set. Thus, making the amounts ambiguous to interpret.
>
> Furthermore it provides only a basic workflow process consisting of
> (approve, reject and review). Associating a party to a budget request
> doesn't warrant that others can not manipulate the content/data associated
> with the request. When I approved a budget request, I had the button to
> reject available. This shouldn't be possible. And at one occasion I could
> click the 'reject' button multipe time to get to the final state.
> While status changes are registered, it should also show who invoked the
> status change.
>
> Due to the fact that arbitrarily any party can be associated to a budget
> request without any restrictions, the workflow is rendered meaningless.
>
> Conclusion(s):
> This solutions hasn't been thought through properly and does no good for
> the quality of OFBiz (both the project and the product) in general and for
> the accounting module in particular. Prior to dumping the code into trunk
> it should have been posted for review by the community, so this could have
> been avoided.
>
> If any other contributor to this project made such a set of functionalities
> available as a patch to an OFBiz issue in JIRA, it would sit there for a
> great length of time before it would have been committed.
>
> I advice to remove it from trunk.
>
> Regards,
>
> Pierre Smits
>
> *ORRTIZ.COM <http://www.orrtiz.com>*
> Services & Solutions for Cloud-
> Based Manufacturing, Professional
> Services and Retail & Trade
> http://www.orrtiz.com
>