You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Pierre Smits <pi...@gmail.com> on 2014/08/10 00:24:31 UTC
Re: Review of Accounting/Budget functionality via r1573884 & r1574400
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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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
Posted by 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.
>>>